-
Notifications
You must be signed in to change notification settings - Fork 76
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 file uploads on localckan #213
Conversation
@wardi are we really wanting to run tests on old ckan versions or should we just update them as well ? |
We can drop tests on old ckans for sure |
If using @wardi I was unable to find whatever fix we may have put in our fork of CKAN Core to fix this issue. Not sure if this was ever an issue with CKAN 2.9 or not? I will keep looking a bit more |
@JVickery-TBS Not sure about requirements. If you're using LocalCKAN then you need to have ckan in the same venv so we could assume werkzeug is there, and is the correct version for the ckan you're running. If you're only using ckanapi for RemoteCKAN we don't need it. If we did add a werkzeug requirement it would have to be an optional dependency: |
@wardi ahh okay makes sense. How does the RemoteCKAN send files? Just as a normal http POST? AHA! I found it, and my original thought was right about the bool method of the https://github.com/ckan/ckan/blob/2.9/ckan/lib/uploader.py#L263
if upload_field_storage is a Looks like the CloudStorage's plugin code does this:
So it is not doing bool on the |
Remote CKAN uses HTTP api and that works even now, you just have to deal with api tokens which is not ideal if you are adding files to a ckan running locally. I'll update the workflows to run on supported ckan versions. |
Done. Cloudstorage and similar do checks on |
Fixes #211
cgi.FieldStorage
has been dropped from ckan since python 3 migration and its going away completely in python 3.13, this just replaces it with werkzeug FileStorage which ckan uses in the core as well.