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

Properly test packages, add galaxy-test-tools and galaxy-model-tools package #8480

Closed
wants to merge 7 commits into from

Conversation

mvdbeek
Copy link
Member

@mvdbeek mvdbeek commented Aug 19, 2019

Builds on #8358

With `pip install -e` the installed packages were just links to
the corresponding source code.

This will highlight a number of issues with some packages.
@mvdbeek mvdbeek force-pushed the properly_test_packages branch 2 times, most recently from 1ecf289 to 00e8a4f Compare August 19, 2019 17:52
@galaxybot galaxybot added this to the 19.09 milestone Aug 19, 2019
@mvdbeek mvdbeek force-pushed the properly_test_packages branch 2 times, most recently from 5ad4be7 to c38597e Compare August 19, 2019 18:46
@@ -55,7 +55,7 @@ def get_var(var_name):
'galaxy.tools.error_reports',
'galaxy.tools.expressions',
'galaxy.tools.filters',
'galaxy.tools.imp_exp',
'galaxy.model_tools.imp_exp',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'galaxy.model_tools.imp_exp',

and add galaxy-model-tools to packages/app/requirements.txt

@jmchilton
Copy link
Member

Fascinating... wasn't a direction I was expecting but looks good.

@jxtx
Copy link
Contributor

jxtx commented Aug 20, 2019

Cool, but why "model_tools". The name is not immediately intuitive to me.

@mvdbeek
Copy link
Member Author

mvdbeek commented Aug 20, 2019

No good reason, we've been calling them that way in the codebase, they're also known as Database Operation tools. We could also call them "special tools", "framework tools" or something else altogether. Do you have a suggestion ?

@jmchilton
Copy link
Member

This will break import/export jobs that occur during an upgrade I think. We broke those last cycle and no one noticed. The little we tried to do to save them just turned out to be unneeded complexity and wasted effort IMO - so I'd just like to do that one more time and keep this as is without worrying about these potential jobs.

@mvdbeek
Copy link
Member Author

mvdbeek commented Aug 20, 2019

I think we may be able to rescue those jobs if we provide a fallback import at the old location -- that should be possible, right ? In tools/__init__.py

from galaxy.model_tools import imp_exp  # noqa: whatever the right number is  # fallback for old imp/exp jobs

@jmchilton
Copy link
Member

I mean - we could definitely fix them. I don't think it is worth adding the lines of code to do that though. It will likely not really save a job across all Galaxies everywhere IMO.

@jxtx
Copy link
Contributor

jxtx commented Aug 20, 2019 via email

@martenson martenson modified the milestones: 19.09, 20.01 Aug 22, 2019
@martenson martenson modified the milestones: 20.01, 20.05 Dec 12, 2019
@mvdbeek mvdbeek modified the milestones: 20.05, 20.09 Apr 20, 2020
@mvdbeek mvdbeek closed this Sep 8, 2020
@jmchilton
Copy link
Member

😿 this was cool stuff - hopefully we find some time to keep it going at some point.

@mvdbeek mvdbeek removed this from the 20.09 milestone Sep 8, 2020
@jmchilton jmchilton mentioned this pull request Apr 5, 2021
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants