10982 10909 Allow using OAI-PMH identifiers as persistent ids of harvested datasets #11049
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…arvested datasets. #10909. (that whole block of extra checks on the harvest "style" may be redundant by now - I'll think about it)
…ed in this branch.
…r the pid of the imported datasets - merging 2 different approaches implemented in the PRs
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
pdurbin
changed the title
pdurbin
changed the title
pdurbin
reviewed
Nov 25, 2024
Member
pdurbin
left a comment
pdurbin
left a comment
There was a problem hiding this comment.
Overall, looks great. Here's some initial feedback.
src/main/java/edu/harvard/iq/dataverse/harvest/client/HarvestingClient.java
Outdated
Show resolved
Hide resolved
src/test/java/edu/harvard/iq/dataverse/api/imports/ImportGenericServiceBeanTest.java
Show resolved
Hide resolved
Co-authored-by: Philip Durbin <[email protected]>
Co-authored-by: Philip Durbin <[email protected]>
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
…added when resolving a conflict with a cherry-picked commit, which of course changes the checksum)
This comment has been minimized.
This comment has been minimized.
pdurbin
approved these changes
Nov 25, 2024
Member
pdurbin
left a comment
pdurbin
left a comment
There was a problem hiding this comment.
I didn't run the code but it looks good (docs too) and the "how to test" instructions seem quite clear. Approved.
This comment has been minimized.
This comment has been minimized.
|
📦 Pushed preview images as 🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name. |
Contributor
|
tested and passed in internal - 1670 Files were successfully harvested |
landreev
added a commit
that referenced
this pull request
May 30, 2025
…led the 2 implementations of the OAI-identifiers-as-global-ids feature, mine and Steven Ferey's. #11479
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
This PR reconciles the 2 implementations of the same feature, mine and @stevenferey 's, hence 2 issue numbers in the PR name. It will only close one of them, #10982; the other issue needs more work on other features being added as part of it and it's going back into "on hold" for now.
Which issue(s) this PR closes:
Special notes for your reviewer:
Suggestions on how to test this:
Below is a real life example of an OAI-PMH archive subject to the feature added in this PR.
Create a harvesting client as follows:
harvest url: https://data.progedo.fr/oai
set: None
metadata format: oai_dc
archive type: Generic OAI archive
Most (or all?) of their records will be failing to import when testing with the develop branch prior to this PR. All (or most) should successfully import when testing this branch. As of writing this, having tested just now:
since this is a real, active archive, their holdings are subject to ongoing change; so, your results may vary.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?:
Additional documentation:
Preview docs at https://dataverse-guide--11049.org.readthedocs.build/en/11049/api/native-api.html#create-a-harvesting-client