Repository navigation
Cache readonly improvements - #10007
Conversation
The mystery segfault may be related to bug astropy#9699?
Read-only filesystems do not raise PermissionError; handle this case too.
The segfault does not appear to be part of astropy but the test suite triggers it if not modified; this skips those tests but leaves them in place. The skipped tests basically exist to allow the coverage tools to work on the lock-contention code - the multiprocessing version does test out the capabilities but the coverage tools don't work with multiprocessing.
|
Sorry for lumping the segfault fix in with the readonly fix but I couldn't even run the test suite without the segfault fix. |
|
I confirm that this fixes #10003 |
|
Yay, thanks, everyone! @aarchiba , please let me know when this is ready for a final review. |
|
I am tentatively slating this for 4.0.1 with the possibility of it slipping to 4.0.2 if needed. |
|
I think it's good to go. Let me know if you want me to squash it but maybe it can wait until I fix the mistakes review catches. |
| assert is_url_in_cache(u) | ||
|
|
||
|
|
||
| @pytest.mark.skip(reason="causes mystery segfault! possibly bug #10008") |
There was a problem hiding this comment.
Maybe we do a skipif so this runs for Python 3.8+? Same comment for all the other similar skips.
There was a problem hiding this comment.
Python 3.6 seems to be fine too.
The point of these tests was to ensure that the locking code got registered by the coverage tools - there are multiprocessing-based tests that explore a variety of concurrent paths through the locking code but the subprocesses don't show on the coverage, so I ran some thread-based tests instead to check all my branches were covered. So with the thread-based tests disabled the coverage tools will see less, even apart from the code that's skipped. Conditional skipping should help a little.
pllim
left a comment
pllim
left a comment
There was a problem hiding this comment.
LGTM! And I don't think this needs squashing. Thanks again.
|
4.0.1 is perfectly fine, I was actually hoping to get these fixes in. |
Fixes #10003
Description
This pull request is to address the cache code's failure to recognize read-only filesystems as being read-only (rather than generically broken).