maint: req py36+ and jh 1.5.1+, fix tests, add RELEASE.md, add pre-commit hooks, add dependabot #273
Conversation
consideRatio
force-pushed
the
pr/general-maint
branch
2 times, most recently
from
99f20c9
to
bc3248e
Compare
consideRatio
force-pushed
the
pr/general-maint
branch
5 times, most recently
from
29fa83f
to
6052f1a
Compare
consideRatio
force-pushed
the
pr/general-maint
branch
from
63950c3
to
479dcce
Compare
for more information, see https://pre-commit.ci
consideRatio
changed the title
|
Ping also to @rkdarst who has contributed a lot to this repo! |
There was a problem hiding this comment.
Thanks for doing this @consideRatio !
I personally am okay with pushing minimum dependencies forward, but system-python users on older distributions may feel differently. I defer to @mbmilligan, who also worked on the test matrix some months back.
.github/workflows/test.yaml
Outdated
| matrix: | ||
| include: | ||
| # test oldest supported version | ||
| - python-version: "3.7" |
There was a problem hiding this comment.
When @mbmilligan was working on #252, I think the user community mentioned that 3.6 was the default python on the Red Hat distributions they were using. I believe the feedback was they tended to use the system python rather than containers or virtual environments. It may have been discussed at the Sep. 2022 meeting.
I don't know if anyone affected by bumping minimum python would be okay with not being able to use the latest batchspawner. However this change is reasonable to me since it is feasible to run jupyterhub in a container or virtual environment on top of older distributions/kernels.
There was a problem hiding this comment.
Ah, I found these notes:
For reference, Python 3.7 reaches end-of-life state 28th June.
There was a problem hiding this comment.
Given most HPC systems run some variant of CentOS, and RH technically provides security support for packages in it until EOL, https://endoflife.date/centos is also relevant. And that ends in 2024. And then I think there's going to be a fairly delayed migration, as IBM bought RedHat and totally fucked everything up over the last few years :D
So I think it would be useful to not drop 3.6 support as part of this PR, because batchspawner / HPC systems have unfortunately different needs than the rest of the ecosystem. I'm very much pro dropping 3.6 support, just in a separate PR to unblock merging this one.
|
@consideRatio |
|
Hi @mbmilligan I hope you are well! Do you think you will find time to review this PR and others to get us towards a release? I also wondered if you feel that its fine if someone else in the jupyterhub team reviewed and merged some PRs? |
|
I would *absolutely* welcome other folks familiar with the codebase helping
out with reviews and testing of PRs. I prefer to have a single person
actually pushing the merges, but I could do that quickly if given good
documentation that proposed changes are okay to merge.
…On Tue, Sep 19, 2023, 6:24 AM Erik Sundell ***@***.***> wrote:
Hi @mbmilligan <https://github.com/mbmilligan> I hope you are well!
Do you think you will find time to review this PR and others to get us
towards a release? I also wondered if you feel that its fine if someone
else in the jupyterhub team reviewed and merged some PRs?
—
Reply to this email directly, view it on GitHub
<#273 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACR7QZ5S322FUCHDY4XH3P3X3F6GPANCNFSM6AAAAAAZLSWLDY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
There was a problem hiding this comment.
As someone who doesn't run HPC systems nor has to maintain anything on CentOS (praise be!), I'm happy to merge this PR if python 3.6 removal is split out into a different PR. I would like someone from batchspawner / HPC community to make that choice as I don't feel qualified to make that.
Thank you so much for working on this, @consideRatio
|
Upon re-reading @mbmilligan's comment in #273 (comment), let me amend to say that I'm very happy to approve all the changes except for 3.6 support dropping. My interpretation of the comment is that @mbmilligan would still prefer to be the one pushing the merge button so am happy to leave that up to them :) |
consideRatio
changed the title
For reference note that JupyterHub 3 and 4 requires python 3.7, so batchspawner of a modern version on python 3.6 will probably use JupyterHub 1 or 2.
|
Thanks for reviewing @yuvipanda and @mbmilligan! I've pushed a commit reverting dropping support for python 3.6. For reference note that jupyterhub 3 requires python 3.7, so users with python 3.6 are stuck with jupyterhub 2 for now also. I also pushed a code coverage related config omitting coverage of tests. |
|
I've reviewed and approved this, @mbmilligan! I am not 100% sure if your comment meant you want to be the person hitting merge, so I'll just give this a few days before I hit merge :) |
|
@consideRatio I think you can also have the 3.6 drop as a separate PR! I personally think it's ok to drop that, but I'd like someone from the HPC community to make that choice. We can definitely 100% do so once CentOS EOLs in 2024, but hopefully someone from the community chimes in and says it's ok to do now :D |
|
We use batchspawner on our HPC platform and well, we dont really rely on python distribution shipped out of OS. We install all Jupyter related stack in a separate conda/mamba environment. Moreover JupyterHub 4+ supports only Python 3.8+, so, I do not really see a point on supporting Python 3.6+ for batchspawner. Saying so, I agree with @yuvipanda that we should drop support for Python 3.6 in a separate PR. This is due to the fact that there is no release of batchspawner that supports JupyterHub 3.* (#277) which supports Python 3.7. So, I think it makes sense to make a release that is compatible with JupyterHub 3.* and then drop support for the next releases that are meant for JupyterHub 4.*. |
|
@mbmilligan are you ok with me merging this PR now? |
Thanks for the ping and for the work pushing this forward. Merging now. |
That was my thinking as well. I'll merge this PR now and set up a release, and after that release we can drop additional support lines. |
|
Thank you for the merge, @mbmilligan! And thank you for doing all this work, @consideRatio! |
This PR is making some progress towards general maintenance needs tracked in #270, doing things done for other repos in the jupyterhub org.
It is a large PR that combines various things that could be made into at least some separate PRs, but has for now been combined to avoid merge conflicts. If its too much work to review in this form or needs various updates, I'll try to extract a few PRs to make it easier to review going onwards.