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

handle missing resources in case we have a race condition with the Da… #4918

Merged
merged 1 commit into from Apr 3, 2020

Conversation

ThrawnCA
Copy link
Contributor

…taPusher, github #3980

It is possible for a resource to be deleted by the DataPusher at the point where we're trying to view it. This pull request handles the error and treats it like a non-datastore resource.

Features:

  • includes tests covering changes
  • includes updated documentation
  • includes user-visible changes
  • includes API changes
  • includes bugfix for possible backport

@boykoc
Copy link
Contributor

boykoc commented Mar 30, 2020

@amercader @ThrawnCA I can confirm that this patch fixes my experience with issue #4287

Although, I'm kind of feeling like this should be addressed somewhere else in datapusher or datastore. When a resource is re-submitted and being populated, why is the datastore_active flag set? Or whatever is deleting the record to re-submit it should set the proper "flags" to prevent things from trying to call it. (not sure yet exactly where the "delete" is happening though...)

It seems in the templates there's lots of checking for datastore_active or if there's a view and both seem to stay as True, which results in trying to load a non-existent datastore record for the resource. The templates chase down from package/resource_read, to package/snippets/resource_view, to package/snippets/resource_view_filters.html until they crash out trying to load a datastore record that isn't in the datastore because it's deleted and being resubmitted.

@amercader
Copy link
Member

Thanks @ThrawnCA , merged and backported. I just added a small change to use core exceptions rather than the toolkit ones (493d601) can you confirm that still works as expected?

@ThrawnCA
Copy link
Contributor Author

ThrawnCA commented Apr 4, 2020

@amercader Thanks for that. I won't be able to test until Monday GMT+10 at the earliest, but it looks like those are just different references to the same exception, so it should be fine.

@SakshiSharma-India
Copy link

Hi @amercader ,
I have faced the similar problem in ckan version 2.7.2.
When I keep on adding large number of resource files one by one into the dataset, I get the below error for some of the resource files:
error : "ERROR [ckan.logic] Could not find resource <resource_id> after all"
And observed that the resource files with resource id mentioned in above error log disappeared from ckan GUI.

Is it possible that above error occurs due to race condition? If yes, I would like to confirm if it is possible that this PR is applicable to ckan v 2.7.2?
We have raised the similar issue #5298

@amercader
Copy link
Member

@SakshiSharma-India you should upgrade to 2.7.7 which is backwards compatible and includes this patch. The latest patch release is the only one we support and contains important security fixes.

@SakshiSharma-India
Copy link

Hi @amercader ,
Thank you so much for your quick response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants