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

Allow for passing until param. #56

Merged
merged 6 commits into from
Apr 20, 2022

Conversation

dkinzer
Copy link
Contributor

@dkinzer dkinzer commented Feb 21, 2018

This commit allows the client to harvest a specific range not limited to
from until now.

Addendum:

  • I noticed also that the from time and current until time seem to not include time granularity. Maybe that should be corrected too?

@bibliotechy
Copy link
Member

👍 Looks like a pretty straightforward and useful addition.

@jrochkind
Copy link
Collaborator

I am now a committer/owner of this project.

Happy to merge this if you are still interested -- but it probably needs a test?

@jrochkind
Copy link
Collaborator

One more try, I am a committer/owner, and could find some time to review/merge this is you are still interested, but I think it probably needs a test to be merged. If I don't hear, I may close this as outdated/abandoned, to clean things up. @dkinzer ?

@dkinzer
Copy link
Contributor Author

dkinzer commented Jan 16, 2020

@jrochkind I haven't been the official developer on the harvester for a while now; and the other developer started using python for this step.

However, I am back on that project now. And I do believe this is still useful. If I start using this gem I again, it would definitely come in handy.

Thanks @jrochkind!

@jrochkind
Copy link
Collaborator

In order to be merged, I think we need a test of some kind. Doesn't need to be perfect, but some kind of test that fails without this code there. Are you interested in adding a test, @dkinzer or @bibliotechy ?

I'm definitely in minimal caretaker mode, don't have time to do much code that isn't directly related to my needs, just trying to do a bit of cleanup while I'm stopping in.

barmintor and others added 6 commits April 20, 2022 11:41
- move DIRECTORY_LAYOUT global to OAI::Harvester::Harvest class and make configurable
- delegate record parsing to ListRecords response in harvester
- do nothing with harvested tempfile if no directory storage is configured
- use application exceptions in harvester
This commit allows the client to harvest a specific range not limited to
from until now.
- harvest_time is expected to be a Time, not a String
@barmintor barmintor force-pushed the allow-harvest-with-until-param branch from 29d9cf3 to af6fa62 Compare April 20, 2022 16:22
@barmintor
Copy link
Collaborator

Ok, I've added a test rig for OAI::Harvester and a test case for this change. It was surprisingly involved, but I think this captures what @dkinzer was going for.

@barmintor barmintor merged commit d676c18 into code4lib:master Apr 20, 2022
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.

None yet

4 participants