-
Notifications
You must be signed in to change notification settings - Fork 122
Build System and Versioning changes #239
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Adds a pyproject.toml with build dependencies, so users can also easily do a "pip install" - Update MANIFEST.in file for sdist - add a test_requirements.txt for easy-install of any dependencies necessary for testing - Update README with build instructions Changes in setup.py: - remove Bluegene stuff, it wasn't used anywhere in the code at all - make some cosmetic changes - allow doing things like "sdist" and "clean" without depending on Cython - Depend atleast on Cython 0.29.30 as minimum version now. This way we can mostly make sure that users have recent Cython version to compile Version naming is changed: Make up the pyslurm version from the Slurm Major release (e.g. 22.5) and the current pyslurm patch-level for this major release, so we have for example: 22.5.0 We must make sure (document it) that users don't confuse this with Slurms patch version
that pyslurm wasn't able to be compiled on some kernels. auto pickling may also not be really needed in pyslurm, because by default classes with pointers/structs as attributes aren't generated with pickle support by cython anyway. For more info, check PySlurm#236 Fixes PySlurm#236
libslurm should be used for interfacing with the C-API, libslurmfull is more internal to the Slurm tools itself and cannot be guaranteed to be stable when used externally. No functions from libslurmfull were actually used in pyslurm.pyx so we can safely make the switch now. Also removes a few functions in slurm/extra.pxi, which are in libslurmfull but not used anywhere in the code Fixes PySlurm#209
giovtorres
approved these changes
Jul 24, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏼
tazend
added a commit
to tazend/pyslurm
that referenced
this pull request
Aug 17, 2022
* Rework the build system - Adds a pyproject.toml with build dependencies, so users can also easily do a "pip install" - Update MANIFEST.in file for sdist - add a test_requirements.txt for easy-install of any dependencies necessary for testing - Update README with build instructions Changes in setup.py: - remove Bluegene stuff, it wasn't used anywhere in the code at all - make some cosmetic changes - allow doing things like "sdist" and "clean" without depending on Cython - Depend atleast on Cython 0.29.30 as minimum version now. This way we can mostly make sure that users have recent Cython version to compile Version naming is changed: Make up the pyslurm version from the Slurm Major release (e.g. 22.5) and the current pyslurm patch-level for this major release, so we have for example: 22.5.0 We must make sure (document it) that users don't confuse this with Slurms patch version * Disable the auto_pickle feature which was causing that pyslurm wasn't able to be compiled on some kernels. auto pickling may also not be really needed in pyslurm, because by default classes with pointers/structs as attributes aren't generated with pickle support by cython anyway. For more info, check PySlurm#236 Fixes PySlurm#236 * Use libslurm.so instead of libslurmfull.so libslurm should be used for interfacing with the C-API, libslurmfull is more internal to the Slurm tools itself and cannot be guaranteed to be stable when used externally. No functions from libslurmfull were actually used in pyslurm.pyx so we can safely make the switch now. Also removes a few functions in slurm/extra.pxi, which are in libslurmfull but not used anywhere in the code Fixes PySlurm#209 Co-authored-by: tazend <toni.harzendorf@gmail.com>
tazend
added a commit
that referenced
this pull request
Sep 11, 2022
* Rework the build system - Adds a pyproject.toml with build dependencies, so users can also easily do a "pip install" - Update MANIFEST.in file for sdist - add a test_requirements.txt for easy-install of any dependencies necessary for testing - Update README with build instructions Changes in setup.py: - remove Bluegene stuff, it wasn't used anywhere in the code at all - make some cosmetic changes - allow doing things like "sdist" and "clean" without depending on Cython - Depend atleast on Cython 0.29.30 as minimum version now. This way we can mostly make sure that users have recent Cython version to compile Version naming is changed: Make up the pyslurm version from the Slurm Major release (e.g. 22.5) and the current pyslurm patch-level for this major release, so we have for example: 22.5.0 We must make sure (document it) that users don't confuse this with Slurms patch version * Disable the auto_pickle feature which was causing that pyslurm wasn't able to be compiled on some kernels. auto pickling may also not be really needed in pyslurm, because by default classes with pointers/structs as attributes aren't generated with pickle support by cython anyway. For more info, check #236 Fixes #236 * Use libslurm.so instead of libslurmfull.so libslurm should be used for interfacing with the C-API, libslurmfull is more internal to the Slurm tools itself and cannot be guaranteed to be stable when used externally. No functions from libslurmfull were actually used in pyslurm.pyx so we can safely make the switch now. Also removes a few functions in slurm/extra.pxi, which are in libslurmfull but not used anywhere in the code Fixes #209 Co-authored-by: tazend <toni.harzendorf@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This makes some changes to the build-system / Versioning scheme
Main points:
I put some updated installation instructions in the README.
I decided to not go for the change to ship the c-sources within the
sdist
, I believe right now it might actually be better to still depend on Cython and rather restrict the Version we support (Check here for more info)Version scheme change proposal:
PySlurm's versioning should only include the
MAJOR.MINOR
release version from Slurm (e.g. 22.05), but not include SlurmsMICRO
version. TheMICRO
Version for PySlurm should more be the internal Patch-Level, increased for example when any bugs have been fixed in the code.It shouldn't be related to Slurms 'MICRO' version, because a PySlurm 22.05.X release should work with any Slurm 22.05.X release - as the Slurm C-API is pretty stable within a Major-Release and headers do not change accross patch-level releases. (Put a note in the README).
Other changes:
libslurm.so
instead oflibslurmfull.so
. Nothing fromlibslurmfull.so
was actually used in the code, so we can make a switch now. See commit message for more explanation and libslurm.so vs libslurmfull.so #209 (Closes libslurm.so vs libslurmfull.so #209)It would probably also make sense to backport these changes to the 21.08 branch, which I can do (and perhaps make also an extra Release for it)
Additionally, the PySlurm version on PyPi is pretty old (18.8) - and I'd really like to see the newer versions up there, especially now with adding the
pyproject.toml
. If you want, I could also do this if you give me permission to upload to PyPi and Test-PyPi.Let me know what you think about all of the proposed changes