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

Change the way we're handling dataset resource updates to changed time #2968

Merged
merged 1 commit into from Dec 16, 2019

Conversation

dafeder
Copy link
Member

@dafeder dafeder commented Oct 1, 2019

This PR changes the logic of how DKAN determines whether to create a "revision log" update to a dataset when a resource is changed. We get a lot of extra revisions to datasets based on changes to resources. Rather than trying to check if the update happened during a harvest, this simply checks if the resource revision is more than a minute older than the dataset's updated date. If not, it's not worth recording.

This should provide more consistency and accuracy, and reduce extra revisions created through automated processes.

Finally, this removes the queue functionality and makes the update immediately. If this creates much longer wait times for harvests to finish we can revisit, but as implemented, the queueing defeats the whole purpose of the revision logging! If the logging is queued and happens hours (or days) later, the updated timestamp will be still inaccurate, just for different reasons.

QA Steps

I'm not sure how to QA this beyond the tests. Let's discuss if there are concerns!

@erogray
Copy link

erogray commented Oct 15, 2019

@dafeder does this ticket need more detail?

@dafeder
Copy link
Member Author

dafeder commented Oct 24, 2019

Description added.

@dafeder dafeder requested a review from janette October 24, 2019 16:29
@dafeder dafeder self-assigned this Oct 29, 2019
@dafeder dafeder assigned dharizza and unassigned dafeder Dec 10, 2019
@dafeder dafeder removed the WIP label Dec 10, 2019
@janette
Copy link
Member

janette commented Dec 12, 2019

  • Tested locally via the UI and works but does take a long time.
  • Test automated Run Harvest multistep on staging server

@dharizza
Copy link
Contributor

Merging as @janette already did the testing on dev and everything went fine.

@dharizza dharizza merged commit 4f59abe into 7.x-1.x Dec 16, 2019
@dharizza dharizza deleted the no-queue-dataset-log branch December 16, 2019 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants