Remove web framework dependency from tools #17058
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
davelopez
added
kind/refactoring
nsoranzo
reviewed
Nov 21, 2023
jmchilton
reviewed
Nov 21, 2023
jmchilton
reviewed
Nov 21, 2023
jmchilton
reviewed
Nov 21, 2023
jmchilton
reviewed
Nov 21, 2023
nsoranzo
reviewed
Nov 21, 2023
davelopez
force-pushed
the
remove_web_dependency_from_tools
branch
3 times, most recently
from
3316d84 to
d8dfeac
Compare
nsoranzo
reviewed
Nov 22, 2023
|
If you are not planning to create a new package for cd packages/app/galaxy/
ln -s ../../../lib/galaxy/short_term_storage/to fix the package tests? |
|
I was actually planning to create a new package but with your help (just wrote to you on Matrix at the same time 😆) but I will add the symlink meanwhile 👍 |
nsoranzo
reviewed
Nov 22, 2023
davelopez
force-pushed
the
remove_web_dependency_from_tools
branch
from
5b455a0 to
518e6b8
Compare
nsoranzo
reviewed
Nov 23, 2023
nsoranzo
reviewed
Nov 23, 2023
davelopez
force-pushed
the
remove_web_dependency_from_tools
branch
from
71e1eba to
192e890
Compare
nsoranzo
reviewed
Nov 24, 2023
|
I think I managed to remove all I could spot so opening for review. Is there some known Pythonic sorcery to check there are no more dependencies left? |
Once/if we split out a tools&workflow package from app, package testing should find out... |
nsoranzo
reviewed
Nov 28, 2023
- Use `util.unicodify` in copy_view.mako for consistency with other usages of unicodify in the file. - Move `to_unicode` to the only place it's used under tool/search/__init__.py
The ShortTermStorageManager is a manager and makes sense to have it live under galaxy.managers instead of galaxy.web. It will also help reduce the dependency of the manager modules on web related stuff.
- And rename it to `api_error_to_dict` since it creates dictionary with and not a message string as the previous name might suggest.
Co-authored-by: Nicola Soranzo <[email protected]>
Co-authored-by: Nicola Soranzo <[email protected]>
The ShortTermStorageManager and related classes should live in their own package eventually and not within galaxy.web nor galaxy.managers as the short term storage microservices it's fairly independent and has way less dependencies than those other packages.
Co-authored-by: Nicola Soranzo <[email protected]>
Temporarily include it in app until the STS goes into its own package.
Temporarily until we create its own package.
We need to use url_builder from the request context to be able to generate valid URLs. Hopefully url_for_not_available will help identify any other possible use of url_for that does not fall back into url_builder.
Replace calls to web.url_for with compatible url_builder.
Replace calls to web.url_for with compatible url_builder. Since there is no real API route for those (they live in client controllers) we need to pass the path prefix as name...
Both links `{host}/activate` and `{host}/user/activate` work but the latter seems more explicit about what are we trying to activate.
Co-authored-by: Nicola Soranzo <[email protected]>
Taking the server_starttime from app instead.
Mostly for consistency with other similar functions in the module. Co-authored-by: Nicola Soranzo <[email protected]>
This should fix URL generation for collections by using the URL builder instead of the legacy url_for.
davelopez
force-pushed
the
remove_web_dependency_from_tools
branch
from
3671279 to
4eb22fe
Compare
This package is used by pulsar with still supports Python 3.7
davelopez
force-pushed
the
remove_web_dependency_from_tools
branch
from
8701ff7 to
1ef8e6a
Compare
nsoranzo
approved these changes
Dec 1, 2023
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #17046
This tries to remove several imports of
galaxy.webfromgalaxy.toolsandgalaxy.managers.Most changes involve handling URL generation without directly importing
galaxy.web.url_for.How to test the changes?
License