Conversation
| Note that if the configured archiver only supports archiving a single version, the call may return 409 CONFLICT if/when another version already has a non-null status. | ||
| ======= | ||
| curl -H "X-Dataverse-key: $API_TOKEN" -H "Content-Type:application/json" -X PUT "$SERVER_URL/api/datasets/:persistentId/$VERSION/archivalStatus?persistentId=$PERSISTENT_IDENTIFIER" -d "$JSON" | ||
| >>>>>>> refs/remotes/IQSS/develop |
There was a problem hiding this comment.
looks like an unresolved merge conflict above
pdurbin
left a comment
pdurbin
left a comment
There was a problem hiding this comment.
The flyway script version needs to be updated.
src/main/resources/db/migration/V5.10.1.3__8605-support-archival-status.sql
Show resolved
Hide resolved
| } | ||
|
|
||
| public boolean isSomeVersionArchived() { | ||
| for (DatasetVersion dv : dataset.getVersions()) { |
There was a problem hiding this comment.
Does it make sense for this method to be on the DatasetServiceBean?
| } | ||
|
|
||
| public boolean isArchivable() { | ||
| String className = settingsWrapper.getValueForKey(SettingsServiceBean.Key.ArchiverClassName, null); |
There was a problem hiding this comment.
Does it make sense for this method (and the next one) to be on the ArchiverUtil?
There was a problem hiding this comment.
@sekmiller - Maybe? These, and the one above are all used in the dataset-versions.xhtml file so moving them completely to ArchiverUtil (or DatasetServiceBean above) means having to make those classes (with ArchiverUtil not being a bean) accessible in the xhtml and not being able to use settingsWrapper to cache the property values(again because , with ArchiverUtil is not a bean/ is called from the API where there is no view/session scope).
One way I could simplify a bit would be to shift the reflective call logic into ArchiverUtil, e.g. a getArchiverOption(String className, String methodToInvoke) method. That would reduce duplicate code and minimize what's in DatasetPage.
Does that make sense? Or is it worth moving them out of DatasetPage completely?
There was a problem hiding this comment.
Maybe just isSomeVersionArchived which is in the DatasetPage as well as Admin and Datasets.
There was a problem hiding this comment.
or maybe it's not worth the trouble
There was a problem hiding this comment.
I'll do some quick de-duplication and push it back. Thanks!
There was a problem hiding this comment.
I moved the guts of someVersionArchived to the util but the rest seem hard - there are methods that require a dataset and some that are static, some use settingsWrapper and some settingsServiceBean due to their context (UI or API) so as I tried to refactor I didn't see good opportunities to consolidate.
TDL/7493-improve_BagGnerator_failure_handling
|
I know at standup I said I'd move this over to QA but I just noticed the API tests didn't run so I just pushed a typo fix to enable another run. |
pdurbin
left a comment
pdurbin
left a comment
There was a problem hiding this comment.
Ok, tests are passing now so I'm hitting "Approve" (@sekmiller is the one who did the actual review).
6 participants
What this PR does / why we need it: Implements an S3 Archiver consistent with recent updates to add archival status support, microprifile config of s3 connections, etc.
Which issue(s) this PR closes:
Closes #8749
Special notes for your reviewer: Builds on #8748 - primarily an S3 archiver class and docs with some minor refactoring to pull a few lines of common code into the abstract base class.
Suggestions on how to test this: Follow the configuration instructions to setup an S3 archiver and test it the same way as a local archiver. The same bag/datacite xml files should be sent to the bucket as would have been sent to the local file system.
Does this PR introduce a user interface change? If mockups are available, please link/include them here: no
Is there a release notes update needed for this change?: addressed in #8611
Additional documentation: