Make download fail immediately on nonexistent resources #820
Conversation
Codecov Report
@@ Coverage Diff @@
## master #820 +/- ##
==========================================
+ Coverage 85.00% 85.26% +0.26%
==========================================
Files 61 61
Lines 6201 6293 +92
==========================================
+ Hits 5271 5366 +95
+ Misses 930 927 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
dandi/dandiarchive.py
Outdated
| if strict: | ||
| raise | ||
| else: | ||
| return |
There was a problem hiding this comment.
that's lot of the same use pattern, I didn't realize it would need to be represented in so many places
I wonder if to reduce at least some code duplication could we have smth like
@contextmanager
def maybe_strict(strict: bool):
try:
yield
except NotFoundError:
if strict:
raiseand then here (and elsehwere) have smth like
with maybe_strict(strict):
yield self.get_dandiset(client).get_asset(self.asset_id)?
| else: | ||
| return None | ||
|
|
||
| def get_version_id(self, client: DandiAPIClient) -> str: |
There was a problem hiding this comment.
just noticed that backward incompatible change is introduced -- we aren't using this one anywhere yet?
There was a problem hiding this comment.
No, we're not using that method anywhere. Since the logic in this method is already used by RemoteDandiset to determine its default version, get_version_id() was redundant.
There was a problem hiding this comment.
and may be we better mark it as minor not as patch ?
There was a problem hiding this comment.
The method was never intended to be called by anything other than other methods in the same class hierarchy (and thus it shouldn't have been called by users of dandi-cli). I seriously doubt anyone ever used it.
Closes #817.