galaxyproject / pulsar

Distributed job execution application built for Galaxy
https://pulsar.readthedocs.io
Apache License 2.0
37 stars 50 forks source link

Test and mypy fixes. Add support for Python 3.11 #312

Closed nsoranzo closed 1 year ago

nsoranzo commented 1 year ago

Planemo tests still fail on the py37-install_wheel build, but I don't know enough about those to fix it.

I think these are still good fixes and could probably be merged nonetheless?

jmchilton commented 1 year ago
nsoranzo commented 1 year ago
  • I would like to continue to support Python 3.6 - until it is blocking something important.

For that you'd need to either change runs-on: to ubuntu-20.04 for all tests or use an ad-hoc way to install Python 3.6 on ubuntu-latest (i.e. 22.04) since actions/setup-python doesn't support it there.

* Are pycurl tests not running that ran before - I'm confused by that line.

That line is redundant since pycurl is later installed by tox (test,install_wheel,mypy: -rdev-requirements.txt) and the package has a different name (python3-pycurl) in Ubuntu 22.04.

nsoranzo commented 1 year ago

Should I try to rebase this, given the conflicts generated by https://github.com/galaxyproject/pulsar/pull/315, or just close it?

jmchilton commented 1 year ago

Doh - sorry. I assumed you had given up on the PR because I wanted to support 3.6. If you'd like to rebase that would be wonderful, but I made the mess and I'd be happy to pull out the 3.11 support and the tox refactorings and apply them in isolation.

nsoranzo commented 1 year ago

I can rebase, hopefully in the next days, no worries.

nsoranzo commented 1 year ago

Rebased and ready for re-review.

mvdbeek commented 1 year ago

Is the latest kombu not being compatible with python 3.6 a good enough reason to drop it ? (https://github.com/galaxyproject/pulsar/actions/runs/4678097934/jobs/8286413656?pr=323). That kombu version seems to have some important timeout handling fixes (https://github.com/celery/kombu/releases/tag/v5.2.3)

nsoranzo commented 1 year ago

BTW, the PR title had become misleading (fixed now), this is not removing Python 3.6 support any more given @jmchilton's concerns (I am still obviously in favour of dropping it).

jmchilton commented 1 year ago

It feels like Pulsar has a lot of modalities that won't require that kombu fix so I'm in favor of continuing to support Python 3.6. The whole purpose of Pulsar originally was allowing the execution of Galaxy jobs in places Galaxy couldn't run - I'm not eager to remove environments.