whole-tale / girder_wholetale

Girder plugin providing basic Whole Tale functionality
BSD 3-Clause "New" or "Revised" License
3 stars 5 forks source link

Expose image on export and publish #551

Closed craig-willis closed 1 year ago

craig-willis commented 1 year ago

Problems addressed

Approach

To Test:

Additional notes

codecov[bot] commented 1 year ago

Codecov Report

Merging #551 (8168d88) into master (c7d8afd) will increase coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #551      +/-   ##
==========================================
+ Coverage   92.81%   92.82%   +0.01%     
==========================================
  Files          60       60              
  Lines        4745     4767      +22     
==========================================
+ Hits         4404     4425      +21     
- Misses        341      342       +1     
Impacted Files Coverage Δ
server/lib/exporters/bag.py 100.00% <100.00%> (ø)
server/lib/manifest.py 93.08% <100.00%> (+0.11%) :arrow_up:
server/lib/manifest_parser.py 88.19% <100.00%> (+0.45%) :arrow_up:
server/lib/dataverse/provider.py 95.96% <0.00%> (-0.41%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Xarthisius commented 1 year ago

Works great, code changes LGTM! One issue I have is more a conceptual one: with this change you can get a different bag for the same versionId depending on what you did on the platform. Perhaps we should add imageInfo to the set of fields we use to detect if something "happened" to the Tale and create a new version for a generic export. However, that doesn't solve the issue when user selects specific version to export from the "version history" menu.