Skip to content

Propose new config value to run harvest sources by external apps - #10

Open
avdata99 wants to merge 7 commits into
GSA:datagovfrom
avdata99:allow_to_harvest_externally
Open

Propose new config value to run harvest sources by external apps#10
avdata99 wants to merge 7 commits into
GSA:datagovfrom
avdata99:allow_to_harvest_externally

Conversation

@avdata99

Copy link
Copy Markdown

This setting value establishes a harvest source as inactive and will allow us to simply harvest these marked sources to harvest externally

@avdata99
avdata99 requested a review from adborden January 22, 2020 20:18

@adborden adborden left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we modify the dataset state? I would expect that an inactive harvest source with a non-CKAN harvest_app would not be harvested either by CKAN or an external harvester. Seems instead of mutating state, we should be hooking into the harvest process and keying off of harvest_app then.

Comment thread ckanext/harvest/plugin.py Outdated
Comment thread ckanext/harvest/plugin.py Outdated
@avdata99

avdata99 commented Jan 23, 2020

Copy link
Copy Markdown
Author

Why do we modify the dataset state? I would expect that an inactive harvest source with a non-CKAN harvest_app would not be harvested either by CKAN or an external harvester. Seems instead of mutating state, we should be hooking into the harvest process and keying off of harvest_app then.

If I just hook into the harvest process we continue needing CRON (or some tool) to start the harvest process. I think is better to let external apps to start the harvesting process in its own way.

Also, If I mutate the state the current jobs will stop automatically. This is good.

What do you think @akariv?

avdata99 pushed a commit to avdata99/ckanext-harvest that referenced this pull request Jun 9, 2020
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.

2 participants