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

Updating the Zenodo handling #162

Merged

Conversation

davidorme
Copy link
Collaborator

@davidorme davidorme commented Jul 3, 2024

This PR expands a bit of the actual issue described in #161 after feedback from Sam.

  • Sam's error came from using outdated/incorrect URLs for the Zenodo API endpoints. I've done two things:
    • Removed the Zenodo API endpoints from the configuration file and hard-coded them. Seemed oddly fussy and not something that changes (often!) and this avoids a simple configuration issue.
    • Updated the Zenodo error handling to report some more basic information and then adding additional JSON data only if available (not all zenodo responses provide additional JSON info),
  • I've added mutually exclusive --live and --sandbox switches to the Zenodo commands to allow data managers to switch the target publication site without having to maintain different config files.
  • I've update the "Using safedata" section for Data Managers to include separate sections on validation, publication and posting metadata, with more discussion of the key points and more example outputs.
  • Updated the code to handle the change in the new version generation on Zenodo - this has shifted from creating an empty deposit from the concept ID to creating a clone from the most recent record ID. The publish_dataset function now does some filename and hash checking to work out how to align the files being provided with what is already online. Docs updated to reflect this.

Closes #161

@davidorme davidorme linked an issue Jul 3, 2024 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Jul 3, 2024

Codecov Report

Attention: Patch coverage is 13.60544% with 127 lines in your changes missing coverage. Please review.

Project coverage is 77.88%. Comparing base (9324649) to head (9e56bf1).
Report is 20 commits behind head on develop.

Files Patch % Lines
safedata_validator/zenodo.py 5.51% 120 Missing ⚠️
safedata_validator/entry_points.py 65.00% 7 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #162      +/-   ##
===========================================
- Coverage    79.04%   77.88%   -1.17%     
===========================================
  Files           13       13              
  Lines         3741     3807      +66     
===========================================
+ Hits          2957     2965       +8     
- Misses         784      842      +58     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@davidorme davidorme requested a review from jacobcook1995 July 3, 2024 15:20
@davidorme
Copy link
Collaborator Author

@jacobcook1995 I haven't tested this stuff, but it does it look sensible? Still need to figure out what went wrong with Sam.

Copy link
Collaborator

@jacobcook1995 jacobcook1995 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, looks sensible to me. Had one minor comment

safedata_validator/zenodo.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@jacobcook1995 jacobcook1995 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a big improvement, just had a few comments

@davidorme davidorme marked this pull request as ready for review July 5, 2024 19:19
@davidorme
Copy link
Collaborator Author

@jacobcook1995 This is changed again with the update to match the changed Zenodo new version process. I think this is a sensible resolution (use md5 and filename to check for new/changed/unchanged/removed files) and I've updated the docs to match.

Copy link
Collaborator

@jacobcook1995 jacobcook1995 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've look through the changes and they all look good to me!

@davidorme davidorme merged commit 9f8e392 into develop Jul 8, 2024
12 checks passed
@davidorme davidorme deleted the 161-cleaner-error-handling-for-create_deposit-entry-point branch July 8, 2024 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cleaner error handling for create_deposit entry point
3 participants