Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PXD-1347 ⁃ use flask restful to consolidate routing #170

Open
rudyardrichter opened this issue Jul 31, 2018 · 2 comments
Open

PXD-1347 ⁃ use flask restful to consolidate routing #170

rudyardrichter opened this issue Jul 31, 2018 · 2 comments

Comments

@rudyardrichter
Copy link
Contributor

rudyardrichter commented Jul 31, 2018

Instead of the current approach with generating blueprint routes at startup using functions like this, we should use flask-restful to consolidate these and eliminate the hacks around setting up the blueprints.

@philloooo philloooo changed the title use flask restful to consolidate routing PXD-1347 ⁃ use flask restful to consolidate routing Jul 31, 2018
@kulgan
Copy link

kulgan commented Aug 2, 2018

I would also suggest certain functionalities are broken up so they can be clearer to work with. The Transaction classes are convoluted and intertwined with the actual functionality being implemented. For example ReleaseTransaction, SubmissionTransaction. Those are always 2 different features, of which one is relatively constant (ie Transaction) being coupled into a set of classes. These can be simplified using a simple context manager for handling transaction which remains constant and reusable then developers can focus on the actual functionality being implemented, eg release, submit, rollback etc. Also we need to consider breaking the project into a reusable library and the actual application that way we don't import an entire application into other projects say gdcapi, only the library gets imported and gdcapi can inherit from the library in ways that does not affect other people's use cases and possibly remove flags

@rudyardrichter
Copy link
Contributor Author

these sound like good ideas :) could you maybe make some separate, more granular issues for these? I think the scope of this issue won't get into the transaction classes.

Also you bring up a good point about separating the application itself. I think that would be simple enough to do just by modifying api.py to return the app through a function instead of exporting it directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants