Conversation
…g client calls (the only thing I want to add is an option of enabling this setting for specific clients; similarly to how ingest size limits can be for all, or some specific formats only. #11473
This comment has been minimized.
This comment has been minimized.
Resolved merge conflicts in: src/main/java/edu/harvard/iq/dataverse/settings/SettingsServiceBean.java src/main/java/edu/harvard/iq/dataverse/util/SystemConfig.java
This comment has been minimized.
This comment has been minimized.
|
Un-drafting this thing. |
|
(will sync w/ develop shortly) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Synced the branch with develop just now. There were no merge conflicts to resolve however, contrary to what GitHub was saying. 🤔 |
src/main/java/edu/harvard/iq/dataverse/harvest/client/HarvesterServiceBean.java
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
…m/IQSS/dataverse into 11473-harvesting-client-ratelimit
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.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@stevenwinship Thanks for adding the test - looks good. |
Co-authored-by: Philip Durbin <philip_durbin@harvard.edu>
|
I just killed and deleted the latest Jenkins built, since it was triggered by a change in the release note. |
|
📦 Pushed preview images as 🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name. |
|
If you add the setting via the api with a bad json file you still get a success message "Setting :HarvestingClientCallRateLimit added", but when you run the harvest you get the "Disabling all harvesting client delay intervals completely until fixed!" . in the log. Is there any way that we can note this on setting create or put the error in the UI on Harvest instead of just in the log? |
|
Hmm. I based this implementation on, or copied it from, how Oliver implemented a similar map setting for tabular ingest size limits. So yes, it is possible to create it with junk values that are only validated when used; similarly to I couldn't think of a completely trivial/cheap way to quickly add validation on create. So I'd like to petition to let it slip. On the rationale that this one is an even more exotic feature than the tab. ingest limits; and something only sysadmins - advanced users, by definition - will have to tinker with; very rarely, if ever. Do we ever validate any other settings on create though?
|
What this PR does / why we need it:
This is based on a patch that I made a while ago for another Dataverse instance. But it has come handy here at HDV and it may be of benefit to other instances out there.
The changes are quite straightforward.
From the accompanying release note:
A setting has been added for configuring sleep intervals between OAI calls for specific harvesting clients. Making it possible to harvest uninterrupted from servers enforcing rate limit policies. See the configuration guide for details. Additionally, this release fixes a problem with harvesting from DataCite OAI-PMH where initial, long-running harvests were failing on sets with large numbers of records.
Which issue(s) this PR closes:
Special notes for your reviewer:
Suggestions on how to test this:
Create a harvesting client and harvest something. For example, the
controlTestSetfrom demo. Confirm that it's working without this new, optional setting.Create the setting as described in the guide. If you configure an interval for the client you're testing, there will be an .info message in the log that the specified number of milliseconds will be used as the delay. If you specify it for a different client, confirm that the client you're testing is unaffected and the delay utilized is 0; i.e. no delay.
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: