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

Move API services to their own package #12548

Merged
merged 9 commits into from
Sep 26, 2021

Conversation

davelopez
Copy link
Contributor

This should tidy up a bit the manager modules and the codebase around the API.

In order to share the controller logic between the legacy UWSGI controllers and the FastAPI controllers during the migration, I started creating these Service classes that wrapped the actual logic for every endpoint operation. These classes were usually living in the same module as the corresponding manager, but they clearly were more API-oriented so now they live in their own package under lib/galaxy/webapps/galaxy/services/.

The idea of wrapping the route operation logic in these classes is to keep the actual API route operation as thin as possible and concentrate on adding the FastAPI stuff (like OpenAPI route documentation) in the lib/galaxy/webapps/galaxy/api/ package. Hopefully, this will mean that if there is a new fancy API framework in the future we can directly reuse those services (actual galaxy logic) and just focus on how to provide the inputs and outputs as the framework likes it.

Apart from just moving around some code, the older Services have been unified a bit to follow the newest approach like avoiding dependencies on app and some general cleanup.

How to test the changes?

  • This is a refactoring of components with existing test coverage.

License

@github-actions github-actions bot added this to the 22.01 milestone Sep 24, 2021
@davelopez davelopez added the kind/refactoring cleanup or refactoring of existing code, no functional changes label Sep 24, 2021
@jmchilton
Copy link
Member

I remember debating with Carl years ago what belonged in the manager layer - I wanted it to do more as a purely pragmatic matter and he wanted it to just deal with model objects and not deal with encoding and decoding IDs for instance - which I agreed was better from a design perspective but I was worry about duplication it would result in.

This separation that you've pioneered is wonderful and really illustrates that the right solution was two layers and this separation. I think splitting the services out into the webapps is perfect and really reflects nicely that they are dealing with web concerns and helps remove these concerns from galaxy-app. This is really wonderful, thank you so much!

@jmchilton jmchilton merged commit 3a57f21 into galaxyproject:dev Sep 26, 2021
@davelopez davelopez deleted the move_api_services branch September 27, 2021 10:21
@davelopez
Copy link
Contributor Author

Thank you so much, John, for all the reviews and support! I really appreciate it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/API area/libraries Data libraries kind/refactoring cleanup or refactoring of existing code, no functional changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants