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

resource_patch does not update file size #7075

Closed
bzar opened this issue Sep 13, 2022 · 3 comments · Fixed by #7103
Closed

resource_patch does not update file size #7075

bzar opened this issue Sep 13, 2022 · 3 comments · Fixed by #7103
Assignees

Comments

@bzar
Copy link
Contributor

bzar commented Sep 13, 2022

CKAN version
2.9.5, probably others

Describe the bug
Updating an uploaded resource file through API does not update the resource's size field

Steps to reproduce
Steps to reproduce the behavior:

  1. Create a dataset and a file upload resource
  2. Observe the resource's size field value
  3. Change the file through the UI
  4. Observe the resource's size field value changes as it should
  5. Use the action API to upload a new file with resource_patch (or resource_update, I assume)
  6. Observe the resource's size field value stays the same

Expected behavior
The file size should reflect the uploaded file regardless of the method used to upload it

Additional details
I assume this is because the resource already has a size field when resource_update calls package_update, which assumes it is already set to the correct value. In resource_create the size is set, but resource_update just keeps the same value as before. Thus the new file size is not set.

@amercader
Copy link
Member

amercader commented Sep 13, 2022

Looks like we need to remove the if 'size' not in resource here?

if 'size' not in resource and 'url_type' in resource:

Do you want to give it a go and submit a PR if it works @bzar ?

@amercader amercader self-assigned this Sep 13, 2022
@robin-NEC
Copy link
Contributor

robin-NEC commented Sep 14, 2022

Hi @amercader @bzar , I would like to work on this issue.

@bzar
Copy link
Contributor Author

bzar commented Sep 23, 2022

@amercader seems to fix the issue with a quick test. Since @robin-NEC has not PR'd yet I'll do it.

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 a pull request may close this issue.

3 participants