IQSS-5505 - only update DOI metadata at PIDprovider when it changes #5506
Conversation
There was a problem hiding this comment.
@qqmyers we added this endpoint specifically for situations where we know we have to update the target url of all of our dvObjects(when we changed the base url of Dataverse. My concern is that having the call to the DataCite api to retrieve and compare the metadata would double the hits to DataCite. Would it make sense to add a flag that would allow you to bypass the compare when you know that update all is required? Or maybe we don't need to be concerned about hitting their api multiple times.
Thanks.
qqmyers
changed the title
|
@sekmiller - maybe some renaming/refactoring is in order: right now there are /modifyRegistration and /modifyRegistrationAll calls that invoke UpdateDatasetTargetURLCommand which updates both the targetURL and metadata (two API calls) without checking the current values (I haven't changed these). Then there are the /modifyRegistrationMetadata and /modifyRegistrationPIDMetadataAll calls which invoke UpdateDvObjectPIDMetadataCommand which also updates the targetUrl and metadata, which I've modified to check current values (just for DataCite). If a fast way to modify just the targetURLs is needed, the former could drop its second call to replace the metadata. Or we could rename these two sets of calls to be the always modify and modify if needed calls. Or both - make one set just push a new target while the other updates targetUrl and/or metadata as needed. I'm open to these or other options - just let me know what fits the use cases best. |
5505-Update_modifyRegistration_API_calls_to_only_update_when_necessary Conflicts: src/main/java/edu/harvard/iq/dataverse/DOIDataCiteRegisterService.java src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDvObjectPIDMetadataCommand.java
|
Can one of the admins verify this patch? |
|
Note in #6845 - @landreev comments just mentioned not updating file DOIs on version updates if that's not needed. This PR would do that. The discussion that has this PR in limbo is about how to expose this capability in the API. That's potentially separable, but also something we could probably figure out an answer for without too much trouble. |
…to_only_update_when_necessary
|
I just noticed that @jggautier mentioned the issue that this pull request closes at #5144 (comment) It looks like it has merge conflicts, though. |
…to_only_update_when_necessary
5505-Update_modifyRegistration_API_calls_to_only_update_when_necessary
…gistration_API_calls_to_only_update_when_necessary
| @@ -695,7 +696,14 @@ public Response updateDatasetPIDMetadataAll(@Context ContainerRequestContext crc | |||
| return response( req -> { | |||
| datasetService.findAll().forEach( ds -> { | |||
| try { | |||
| logger.fine("ReRegistering: " + ds.getId() + " : " + ds.getIdentifier()); | |||
| if (!ds.isReleased() || (!ds.isIdentifierRegistered() || (ds.getIdentifier() == null))) { | |||
| if (ds.isReleased()) { | |||
There was a problem hiding this comment.
If this fails because the datasets is not released then nothing will go into the log. Do we want to put something in the logs?
There was a problem hiding this comment.
I guess not, we just want to skip it.
There was a problem hiding this comment.
Yeah - we currently don't update the PID service after every edit. We could but there's also not a lot of value since the PID isn't searchable.
landreev
added
Size: 10
|
Looking good so far! |
New Contributors
Welcome! New contributors should at least glance at CONTRIBUTING.md, especially the section on pull requests where we encourage you to reach out to other developers before you start coding. Also, please note that we measure code coverage and prefer you write unit tests. Pull requests can still be reviewed without tests or completion of the checklist outlined below. Thanks!
Related Issues
Pull Request Checklist