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

Streaming uploads revisited #180

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jqnatividad
Copy link
Contributor

Hi @wardi. Happy New Year!
I adapted @hvwaldow's old streaming uploads PR (#109), and can confirm it works like a charm!

Exercised it heavily with multi-gigabyte uploads both from the CLI and as a python module, and it performs as expected.

Submitting the PR again as I think it will be a valuable addition to ckanapi (and I'd rather not maintain my own fork :) ).

@wardi
Copy link
Contributor

wardi commented Apr 27, 2021

is the progress bar compatible with LocalCKAN (or ckanapi -c)? If not the switch should be moved together with the --insecure flag in the CLI which is only available when using ckanapi -r

@frafra
Copy link
Contributor

frafra commented Feb 1, 2022

Can we move [-P] and accept the change as it is?

@@ -16,8 +16,8 @@ def __init__(self, expected_name, expected_args, response,
self._expected_files = expected_files or {}
self._response = response

def call_action(self, name, args, context=None, apikey=None, files=None,
requests_kwargs=None):
def call_action(self, name, args, context=None, apikey=None, progress=None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new parameters at the end, just in case

@@ -79,7 +97,8 @@ def prepare_action(action, data_dict=None, apikey=None, files=None):
continue # assuming missing will work the same as None
if isinstance(v, (int, float)):
v = str(v)
data_dict[k.encode('utf-8')] = v.encode('utf-8')
#data_dict[k.encode('utf-8')] = v.encode('utf-8')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#data_dict[k.encode('utf-8')] = v.encode('utf-8')

elif type(s) == unicode:
return s.encode('utf-8') # It is PY2 :class:unicode
else:
return s.decode('utf-8').encode('utf-8') #type(s) is PY2<str> or illegal
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this just to raise an error for invalid types?

@wardi
Copy link
Contributor

wardi commented Feb 1, 2022

Would be nice to have a test for this functionality as well.

@ChristianF88
Copy link

Hi guys! I'm a quite new ckan user / dev. And I was wondering if there's any plan to merge this soon? Cheers!

@wardi
Copy link
Contributor

wardi commented Mar 2, 2023

@ChristianF88 there are some small things to fix I pointed out above, and some new conflicts.

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 this pull request may close these issues.

4 participants