Adds option to disable filelock, updates tests (Issue #90) #91
Conversation
This file contains 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
vbharadwaj-bk
changed the title
vbharadwaj-bk
changed the title
tbenthompson
approved these changes
May 29, 2024
README.md
Outdated
| @@ -173,6 +174,35 @@ cppimport.settings['lock_timeout'] = 10*60 # 10 mins | |||
|
|
|||
| You should not use `force_rebuild` when importing concurrently. | |||
|
|
|||
| ### Acquiring the lock hangs or times out unexpectedly - what's going on? | |||
| Certain filesystems do not support file locking. You can disable the lock | |||
There was a problem hiding this comment.
Mind giving an example or two so people might have a better guess of whether their OS supports locking or not?
|
Also looks like the CI flake8 checks are failing on some of the new code. There's a pre-commit config that should get things set up correctly to avoid those issues. but you're also welcome to just flake8 directly. |
|
I also invited you as a collaborator. Once the CI is passing, feel free to merge this when you're ready. |
…ing and flock test.
|
Thanks! Yeah, it seems like we should just remove Python 3.7 from the CI. |
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.
Fixes #90
This PR adds the
use_filelockoption to cppimport's global settings (Trueby default). By setting this toFalse, users are no longer protected from multiple processes competing to rebuild an extension. This is necessary on filesystems that don't support file locking, e.g. [here].(https://docs.nersc.gov/performance/io/dvs/#:~:text=DVS%20doesn't%20support%20file,locking%2C%20please%20use%20Perlmutter%20Scratch.)The README has been updated to describe this behavior. In
test_cppimport.py, all tests EXCEPT the multiprocessing test have been updated to disable the file lock. The multiprocessing test only runs with the invocationpytest --multiprocessing, and it does not disable the file lock to test this feature.Verified that all tests passing (2 warnings). Feedback welcome!