Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make use of /assets/{asset_id}/info/ endpoint #895

Merged
merged 1 commit into from
Feb 2, 2022
Merged

Conversation

jwodder
Copy link
Member

@jwodder jwodder commented Jan 31, 2022

This PR makes use of the /assets/{asset_id}/info/ endpoint added by dandi/dandi-archive#681 to construct BaseRemoteAssets that have all of their "meta-metadata" from their Django serializations, thereby accomplishing the following:

  • Add created and modified fields to BaseRemoteAsset
  • Add BaseRemoteBlobAsset and BaseRemoteZarrAsset subclasses of BaseRemoteAsset with blob and zarr attributes, respectively
    • These subclasses determine the return value of asset_type based on overloading rather than relying on encodingFormat
    • filetree and iterfiles() have been moved from RemoteZarrAsset to BaseRemoteZarrAsset
  • BaseRemoteAsset.from_metadata() has been replaced with BaseRemoteAsset.from_base_data()
  • Support download of Zarrs identified by https://<server>/api/assets/<asset-id>/ URLs (Closes Support downloading Zarrs identified by https://<server>/api/assets/<asset-id>/ URLs #883)

@jwodder jwodder added the minor Increment the minor version when merged label Jan 31, 2022
@codecov
Copy link

codecov bot commented Jan 31, 2022

Codecov Report

Merging #895 (bc29b0c) into master (8d14434) will increase coverage by 0.03%.
The diff coverage is 90.56%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #895      +/-   ##
==========================================
+ Coverage   86.66%   86.70%   +0.03%     
==========================================
  Files          61       61              
  Lines        7268     7282      +14     
==========================================
+ Hits         6299     6314      +15     
+ Misses        969      968       -1     
Flag Coverage Δ
unittests 86.70% <90.56%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
dandi/download.py 86.96% <75.00%> (-0.24%) ⬇️
dandi/dandiapi.py 88.39% <90.90%> (-0.23%) ⬇️
dandi/tests/test_download.py 100.00% <100.00%> (ø)
dandi/dandiarchive.py 84.42% <0.00%> (+2.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d14434...bc29b0c. Read the comment docs.

@yarikoptic
Copy link
Member

Looks LGTM upon cursory review, no tests had to be adjusted (good), and test for new functionality added (good). Thank you @jwodder !

@yarikoptic yarikoptic merged commit 11a95f1 into master Feb 2, 2022
@yarikoptic yarikoptic deleted the asset-info branch February 2, 2022 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support downloading Zarrs identified by https://<server>/api/assets/<asset-id>/ URLs
2 participants