Skip to content

Conversation

tazend
Copy link
Member

@tazend tazend commented Nov 6, 2021

Hi,

this pull request creates the baseline to make pyslurm compile for 20.11 (Specifically, I compiled with 20.11.8, but as usual any 20.11.* patch should work)

  • slurm.pxd now should be again identical to the slurm headers of 20.11 for slurm.h, slurmdbd.h and slurm_errno.h

Also credits to @basvandervlies for modifying the pyslurm.pyx file (provided in #180) adding some of the necessary patches. I included the commit in this PR.

I put some modifications in though:

  • powercap stuff seems to be outdated and can be removed, the functions don't exist anymore in the slurm headers

@giovtorres Let me know what you think and if you are also able to compile it.

I also think for the current master branch with 20.2 support a dedicated 20.2 should be created to have that seperated. (basically in the same way the slurm devs do it on github)

closes #189

Copy link
Member

@giovtorres giovtorres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tazend This looks great! 🎉 And it compiles just fine inside a test Slurm container! Just one small ask to update the version in __version__.py so that it matches the version of Slurm it was compiled against. Then we can ship!

Also, it would be a big help if you could document the steps you took to generate this version. I believe there was a BUILD.md that didn't make it into the main branch. (This doc can be in a follow-up PR.)

@tazend
Copy link
Member Author

tazend commented Nov 9, 2021

@giovtorres Hey, sure I can provide that, makes sense also.

Might probably be worth to put this explanation in a seperate README.md inside the jinja2 directory, to explain the purpose of it, how to "build" the slurm.pxd from the templates and how to go about performing major upgrades pulling in the new header definitions (e.g. using autopxd)

I'll do that in a seperate PR in the next few days.

Copy link
Member

@giovtorres giovtorres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀 Thank you so much for this contribution!!

@giovtorres giovtorres merged commit 6ca5605 into PySlurm:master Nov 10, 2021
@giovtorres giovtorres mentioned this pull request Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to support Slurm 20.11
3 participants