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

Fix deletion in JSON Harvester #157

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

EricSoroos
Copy link

Deletion in the JSON harvester is broken in 2 ways

  • The json harvester crashes in the import stage when it can't access the harvest object id: None, because the id is queued before it is generated in obj.save()
  • All of the harvest objects for the deleted dataset are set to not current before the crash, so the relationship between the dataset and the harvester is lost, so the dataset will live on in CKAN when it has been deleted from the source.

This patch fixes the crash by saving the harvest object prior to using it's id, and only clearing the harvest objects for the deleted item after it has been deleted.

@EricSoroos
Copy link
Author

Looks like tests for ckan=master are failing for unrelated reasons.

@metaodi
Copy link
Member

metaodi commented Jul 9, 2019

@EricSoroos can you write a test for your change?

@EricSoroos
Copy link
Author

Sorry, I can't get the test suite to run locally based on the README:

To run the tests, do:

    nosetests --nologcapture --ckan --with-pylons=test.ini ckanext

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