Make sure fs exceptions are raised if not MissingFs exceptions (clone)
note: This PR is a copy of PR #1237 which had become stale. Description from @martindurant is:
(for v2 for now)
When FSStore gets exceptions, these correctly become the fill value when they are translated to KeyError. However, some exceptions do not equate to missing keys but to other permanent problems such as PermissionError. This change makes sure they are correctly raised to the caller even within getitems (getitem already was correct).
TODO:
- [x] Add unit tests and/or doctests in docstrings
- (NA) Add docstrings and API docs for any new/modified user-facing classes and functions
- (NA) New/modified features documented in docs/tutorial.rst
- [x] Changes documented in docs/release.rst
- [x] GitHub Actions have all passed
- [x] Test coverage is 100% (Codecov passes)
It would be nice to get the workflows authorized so I could see what more to fix and document in release notes.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 99.99%. Comparing base (
2534413) to head (21a5ab7).
Additional details and impacted files
@@ Coverage Diff @@
## main #1604 +/- ##
=======================================
Coverage 99.99% 99.99%
=======================================
Files 38 38
Lines 14580 14604 +24
=======================================
+ Hits 14579 14603 +24
Misses 1 1
| Files | Coverage Δ | |
|---|---|---|
| zarr/storage.py | 100.00% <100.00%> (ø) |
|
| zarr/tests/test_storage.py | 100.00% <100.00%> (ø) |
Failing test is the same seen in #1611, something with Azure connection.
It would be nice to get the workflows authorized so I could see what more to fix and document in release notes.
Apologies for the slowness. It's the end of year crunch. Actions launched.
Failing test is the same seen in #1611, something with Azure connection.
IIUC, this has been happening more frequently recently. A relaunch usually helps. I'll keep an eye on it.
@joshmoore Trying to help with another merge to main. Maybe not helpful though ...
Scan of the 178 failed tests indicates azure.core.exceptions.HttpResponseError from tests WithABSStore are the culprit.
@itcarroll, it looks like re-running the jobs worked!
@joshmoore I know that no issue was created to precipitate @martindurant's original PR, but this fixes an actual, real, silent, bug, so it would be great to finish it off. Thanks!
This has been quite the quest -- thanks @itcarroll for hanging in there. @martindurant - are you happy with this solution? Would love to get this into a release (tomorrow 🤞).