[22.01] Create config package, move app-related functionality to app

[mvdbeek]

Work towards #13451

How to test the changes?

(Select all options that apply)

• I've included appropriate automated tests.
• This is a refactoring of components with existing test coverage.
• Instructions for manual testing are as follows:
  1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

• I agree to license these contributions under Galaxy's current license.
• I agree to allow the Galaxy committers to license these and all my past contributions to the core galaxy codebase under the MIT license. If this condition is an issue, uncheck and just let us know why with an e-mail to galaxy-committers@lists.galaxyproject.org.

[nsoranzo]

Does this need to target 22.01? Seems quite "invasive" so late in the release cycle.

[mvdbeek]

I think so, otherwise we have to hack around the path resolution to the interactivetools_map setting.

[mvdbeek]

Though we could publish the config package from the dev branch and depend on that in gravity.

[mvdbeek]

(It also just moves around a few things around, it doesn't add new code, I don't think the lines changed number gives a good impression of the changes. #13454 was much more invasive in that regard)

[nsoranzo]

Fine by me, #13451 didn't directly mention issues with interactivetools_map , good to have it stated somewhere for future reference!

[jdavcs]

I think so, otherwise we have to hack around the path resolution to the interactivetools_map setting.

If we merge this into 22.01, I'll have to hack around the alembic branch. A lot. Why 22.01? This is a considerable change to the code base (most welcomed and needed, but considerable).

[mvdbeek]

Galaxy depends on gravity. I don't think you can place Galaxy packages in Galaxy's virtualenv if you're running Galaxy from the root tree.

That's a good point, but I'm pretty sure that works and gravity would use the virtualenv package, while a root install uses local imports (I think that's because we set PYTHONPATH). Or so this seems to be the case for planemo installed in a Galaxy virtualenv.

I tested this real quick now by editing version.py to 10.01 in the galaxy-util package, installing the package, then stashing the changes:

$ python -c "from galaxy import version; print(version.VERSION)"
10.01.rc1
$ PYTHONPATH=lib/ python -c "from galaxy import version; print(version.VERSION)"
22.01.rc1

Do you think this is good enough, or do you think we need to add a workaround like pulsar-galaxy-lib ?

[mvdbeek]

Can you please move pykwalify from packages/app/requirements.txt to packages/app/test-requirements.txt ?

Do we need it at all in packages/app, given that app depends on config ?

[nsoranzo]

Can you please move pykwalify from packages/app/requirements.txt to packages/app/test-requirements.txt ?

Do we need it at all in packages/app, given that app depends on config ?

Following "explicit is better than implicit", I prefer we add all packages that are imported in some package B files even if they are already dependency of a requirement A. In this way, if the package A drops the dependency at some point, package B doesn't break.

[mvdbeek]

Uff, that seems hard to maintain as a policy without tooling to enforce this. Are we sure we're doing that now ?

[nsoranzo]

Uff, that seems hard to maintain as a policy without tooling to enforce this. Are we sure we're doing that now ?

As a best effort. We also sometimes miss requirements in pyproject.toml that gets installed as deps of deps.

[jmchilton]

Planemo should never be installed in Galaxy's virtualenv 😅 - this used to crash and burn terribly and that is what I would expect. This seems problematic to me, Python isn't node.

I don't think we should have two sources of truth in Galaxy's virtualenv. I don't think this makes sense for a production setting. Bringing gravity into Galaxy's package system isn't a valid way to deal with this?

[mvdbeek]

this used to crash and burn terribly and that is what I would expect.

I think that wasn't necessarily due to fundamental issues with the approach, but how we propagated the VIRTUAL_ENV variable. But of course it's not a good idea since the non-galaxy dependencies could conflict.

Bringing gravity into Galaxy's package system isn't a valid way to deal with this?

We could of course, but how would this work ? I'd assume we'd still have to install the gravity package ?

[nsoranzo]

Bringing gravity into Galaxy's package system isn't a valid way to deal with this?

I did propose something along these lines: #13224 (comment) but @natefoo was not in favour.

[mvdbeek]

I'd also assume that production setups wouldn't necessarily install gravity into the same virtualenv as Galaxy, we only do that for simplicity in the run.sh case. Maybe we should drop gravity from requirements.txt and install it somewhere in common_startup.sh ?

[mvdbeek]

Alright, so after sleeping a night on this I see a few (not mutually exclusive) options and ramblings:

• Be pragmatic and have IT deployers repeat the absolute path to the session map file. In any case you'll have to set up a proxy and job_conf.xml with a docker-enabled destination, so I guess this doesn't really weigh heavy in terms of having an out-of-the box IT experience. I sort of saw this as a release blocker, but it should probably not be one.
• Go with @jmchilton's suggestion of publishing a dependency-less gravity package that we install in the run.sh scenario. I don't really love that because I think we'll have to hardcode PYTHONPATH in the virtualenv activation script, as otherwise gravity won't find its dependencies (unless you run gravity from galaxy's lib directory). Maybe I'm not thinking of another way to make this work though, so if you have an idea let me know.
• Install all Galaxy packages (with pip install -e ?) into Galaxy's virtualenv and combine that with a dependency-less gravity. That'd solve the issue of how we can use gravity without wrangling PYTHONPATH, but that does actually seem like a pretty big change that I think is indeed too late for 22.01
• Ignore the redundant source of truth issue @jmchilton mentioned for this release and work towards pip install -e all the things for a root layout for 22.05.

I think none of these options would speak against a config package. I'm gonna go ahead with the first option for now, but I'd like for the config package to be in 22.01, even if it won't be used in the gravity version we'll ship with 22.01.

[nsoranzo]

I'd also assume that production setups wouldn't necessarily install gravity into the same virtualenv as Galaxy, we only do that for simplicity in the run.sh case. Maybe we should drop gravity from requirements.txt and install it somewhere in common_startup.sh ?

Maybe we can install gravity with pipx ?

[mvdbeek]

Maybe we can install gravity with pipx ?

That is definitely an interesting option. We'll have to figure out the state dir setting, but if we can do that I think that would be ideal.

[mvdbeek]

Maybe we could ignore the state dir issue for now. The documentation says to activate galaxy's virtualenv, which has a default state dir set. So this would still work, as long as Galaxy's virtualenv doesn't include a conflicting gravity. It does take away from the advantage of using pipx, but it's better than the other possible options at this point. And you can always set the state dir explicitly.

[nsoranzo]

@mvdbeek I am a bit lost with the plan, and I think we need buy-in from at least @natefoo and @jmchilton . Should we discuss this at the next backend-wg meeting on 2022-03-08, but merge this in the mean time?

[jmchilton]

I continue to hate this but I don't understand the underlying issues. We won't really have a very clean way to remove those older Galaxy packages out of the virtualenv if we decide to undo this in a future release. This isn't a -1 though, I hope I prove to be wrong about this - and this is just a once bitten, twice shy kind of response.

[jmchilton]

Also... to be clear... I do love having a standalone galaxy-config package. That is absolutely awesome regardless of the dependency issues with gravity.