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 uploads fail on LocalCKAN #211

Closed
EricSoroos opened this issue Nov 24, 2023 · 1 comment · Fixed by #213
Closed

Resource uploads fail on LocalCKAN #211

EricSoroos opened this issue Nov 24, 2023 · 1 comment · Fixed by #213
Labels

Comments

@EricSoroos
Copy link

EricSoroos commented Nov 24, 2023

I spent last night tracking down a resource upload issue in 2.9 / python 3.8-- and it looks like a combination of Ckan and CkanAPI.

Uploads were silently failing to save with this code:

files = {'upload': open(csv_path, 'rb')}                                                                                                                          
localapi.call_action(resource_patch, res_dict, files=files)

LocalCkan creates a cgi.FieldStorage, and passes it into the action chain, which ultimately creates a ResourceUpload here: https://github.com/ckan/ckan/blob/2.9/ckan/lib/uploader.py#L262.

        if bool(upload_field_storage) and \
                isinstance(upload_field_storage, ALLOWED_UPLOAD_TYPES):

This is checking for truthy file uploads, and one of the allowed types.

(Pdb) ALLOWED_UPLOAD_TYPES
(<class 'cgi.FieldStorage'>, <class 'werkzeug.datastructures.FileStorage'>)
(Pdb) type(upload_field_storage)
<class 'cgi.FieldStorage'>
(Pdb) isinstance(upload_field_storage, ALLOWED_UPLOAD_TYPES)
True
(Pdb) upload_field_storage
FieldStorage(None, '/var/tmp/no-of-inspections-all-types-of-early-years-services-2023.csv', b'AreaID,RegionID,Area,Year,Q1,Q2,Q3,Q4\r\n23,23,National,2023,606,628,,\r\n')
(Pdb) bool(upload_field_storage)
False

cgi.FieldStorage is list truthy (if it's list member is true), but setting the file doesn't add any fields into the list, so it's falsy.
werkzurg.datastructures.FileStorage has different falsy behavior.

I fixed my issue by

                file_name = csv_path.split('/')[-1]
                resc_dict.update({'upload': werkzurg.datastructures.FileStorage(open(csv_path, 'rb'), file_name, content_type='text/csv')})
                toolkit.get_action('resource_patch')(context,
                                                      resc_dict)

So, there are a couple of things.

  1. stdlib cgi is deprecated in 3.13, because there's basically no maintainers and there are weird bugs in it. We are going to eventually need to shift off of it.
  2. werkzurg works as a drop in replacement here, and should be supported going forward. It will certainly work for local ckan at any rate, because we're going to have it installed already for ckan.
  3. Perhaps the truthy test would be better as a is not None in ResourceUpload
@EricSoroos EricSoroos added the bug label Nov 24, 2023
@Zharktas
Copy link
Member

Zharktas commented Mar 8, 2024

Stumbled to this as well, our local file uploads have been failing for awhile and I started looking into it. cgi.FieldStorage should be removed as its there only for backwards compatibility and since ckan 2.8 hasn't been supported for awhile, ckanapi could do without it as well.

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

2 participants