fix: support older versions of python#409
Conversation
- xcollections.pyx: replaced dict | / dict |= with dict(a, **b) (merge, right wins) and .update() (in-place merge) — semantically identical, works on Python 3.6+
- submission.pyx: wrapped Path.is_file() in try/except (ValueError, OSError) — in Python 3.6 a null byte in the path raises ValueError; 3.7+ silently returns False
- test_reservation.py: replaced datetime.fromisoformat("2022-04-03T06:00:00") with datetime(2022, 4, 3, 6, 0, 0) — fromisoformat was added in 3.7
|
@tazend i updated the test fixture to include slurm built on rocky linux versions 8, 9 and 10. On RL8, the default is Python 3.6 and while testing pyslurm against this version, I had to make some changes. Would you mind having a look? |
tazend
left a comment
tazend
left a comment
There was a problem hiding this comment.
Hi @giovtorres,
looks good!
The changes for Python 3.6 compat are fine. In the future however, if there is at any point a feature that we explicitly need to make use of, that is not available in EOL python versions (example: dataclasses), I would honestly not make any workaround attempt and simply drop support for the version. Or in general, I would not put in effort to make sure that code changes in the library are compatible with EOL python versions.
Especially python 3.6 is EOL since the end of 2021. While it is (unfortunately) still the default on RL8, it is easily possible to install more up-2-date python versions through standard repositories (for example, python 3.11 from appstream). So this is in my opinion not really an argument to keep the library compatible in the future.
Just some notes, should not block this current PR though :)
In regards to the slurm_msg_t updates: It is an unfortunate change from upstream Slurm that currently breakes the promise of "Any pyslurm 25.11.x release is compatible with any Slurm 25.11.X release". This change was introduced at Slurm 25.11.3
So basically with this, batch-script retrieval will only work on Slurm Versions starting from 25.11.3. We should definitely do that change regardless of breaking earlier versions, but we might need to look into some sort of "conditional compilation".
We already know at build time which version we target, since we parse the slurm_version.h file, so we can make use of this information.
Cython doesn't really support conditional compilation directly (and deprecated any forms of it), but encourages to rather use templating: https://docs.cython.org/en/latest/src/userguide/tempita.html
Maybe we could introduce something like this for cases where we actually need to do different things depending on the Patch version of Slurm.
2 participants
The recent consolidation of setup.py into pyproject.toml didn't account for python 3.6. This change fixes the package name and version for versions of python that don't support an old enough version of setuptools that in turn supports [project] in pyproject.toml.
Tested locally on Python 3.6 on Rocky Linux 8. Will need to update the test suite to use python 3.6 to prevent regressions.
Bump version to 25.11.2.
Closes #408