Conversation
|
Hi Giovanni, thank you :) I will check it out, but it looks already good - instead of waiting for a hardcoded amount of seconds, it makes more sense to poll whether the correct state of the job is already there or not, so that definitely makes more snese :) |
| docker exec "$CONTAINER_NAME" bash -c " | ||
| cd /pyslurm | ||
| rm -rf build/ *.egg-info | ||
| find pyslurm -name '*.so' -delete |
There was a problem hiding this comment.
The setup.py script pretty much already removes all the *.c and *.so files on every run automatically to get a clean build. In the past I had some suprising effects when not cleaning these files and let Cython decide to only rebuild those that were required to recompile. But many of the files are so interleaved now that almost a full rebuild is necessary anyway when something changes I think :)
| source $VENV_PATH/bin/activate | ||
| pip install -q --disable-pip-version-check -r /pyslurm/test_requirements.txt | ||
| cd /pyslurm | ||
| pip install -v --disable-pip-version-check -e . --no-build-isolation --config-settings='--build-option=build_ext -j$BUILD_JOBS' |
There was a problem hiding this comment.
This would also be covered via the scripts/build.sh script. It additionally sets an environment variable called PYSLURM_BUILD_JOBS that is used in setup.py to run the cythonize command with multiple threads. This is iirc separate from the parallelism achieved via the -j flag to build_ext, which only covers compilation afterwards, but not the cythonize step.
I think I would therefore prefer leveraring the existing build.sh script for any installations. We can of course extend the script with further options that are required but not covered yet - but then we have it in a central place.
| assert job.steps.get("batch") | ||
|
|
||
|
|
||
| def test_load_stats(submit_job): |
There was a problem hiding this comment.
Did this test_load_stats test fail in your testing? At least in my recent testings it was always working.
If it didn't work we can reintroduce it later, so it shouldn't be a blocker here, but its definitely needed :)
|
Also, thank you for completing some tests that were still having a |
2 participants
This change gets the test suite working again on pull request. It starts with just python3.12. The tests now poll instead of sleeping for 3 seconds between some assertions. I also attempted to address some flaky tests that would only sporadically fail when a race condition was met.
I also updated the test suite to use an updated slurm container that has newer versions of slurm installed in a single container.