Add extra data option to add_source #83
Conversation
|
@arlind87 can you help with the build issues on this one? |
@joaquimadraz i will check on this but it seems some of the python versions are depricated and have not worked for a long time |
joaquimadraz
force-pushed
the
extra-data-to-sources
branch
from
dd8030c to
5a96516
Compare
| """Create a new Source on the given dataset and return its URL.""" | ||
| sources_url = ds.user_url.catalogs['sources'] | ||
| # Don't call Catalog.post here (which would force application/json); | ||
| # we want requests.Session to set multipart/form-data with a boundary. | ||
| new_source_url = ds.session.post( | ||
| sources_url, files={"uploaded_file": (filename, fp, mimetype)} | ||
| sources_url, files={"uploaded_file": (filename, fp, mimetype)}, data=data |
There was a problem hiding this comment.
What is this data argument? The POST method on the sources controller does not specify such argument.
There was a problem hiding this comment.
The sources controller now expects data in the POST body which is where we are sending the required attributes
for the crunchlake source.
data is not a sources controller specific attribute. It's request body data for the POST request.
There was a problem hiding this comment.
Ah, got it, this is not a data query param, but the request data
src/pycrunch/importing.py
Outdated
| @@ -53,13 +53,13 @@ def wait_for_batch_status(self, batch, status): | |||
| raise ValueError("The batch did not reach the '%s' state in the " | |||
| "given time. Please check again later." % status) | |||
|
|
|||
| def add_source(self, ds, filename, fp, mimetype): | |||
| def add_source(self, ds, filename, fp, mimetype, data={}): | |||
There was a problem hiding this comment.
Best to default to None - data=None, in this context an empty dict is probably safe, but it is considered to be a bad smell.
There was a problem hiding this comment.
Good catch, definitely a bug waiting to happen
src/pycrunch/importing.py
Outdated
| @@ -53,13 +53,13 @@ def wait_for_batch_status(self, batch, status): | |||
| raise ValueError("The batch did not reach the '%s' state in the " | |||
| "given time. Please check again later." % status) | |||
|
|
|||
| def add_source(self, ds, filename, fp, mimetype): | |||
| def add_source(self, ds, filename, fp, mimetype, data={}): | |||
There was a problem hiding this comment.
Good catch, definitely a bug waiting to happen
| """Create a new Source on the given dataset and return its URL.""" | ||
| sources_url = ds.user_url.catalogs['sources'] | ||
| # Don't call Catalog.post here (which would force application/json); | ||
| # we want requests.Session to set multipart/form-data with a boundary. | ||
| new_source_url = ds.session.post( | ||
| sources_url, files={"uploaded_file": (filename, fp, mimetype)} | ||
| sources_url, files={"uploaded_file": (filename, fp, mimetype)}, data=data |
There was a problem hiding this comment.
Ah, got it, this is not a data query param, but the request data
|
@jjdelc I've changed the default value for data argument |
Enables sending extra body data when creating a source. Useful for the crunchlake source.