-
Notifications
You must be signed in to change notification settings - Fork 7
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
add method to upload sources from schema and metadata #82
Changes from all commits
a4ea5d0
77e2c0d
e550231
8d9e924
f7d0546
01f2a2d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,35 @@ def wait_for_batch_status(self, batch, status): | |
else: | ||
raise ValueError("The batch did not reach the '%s' state in the " | ||
"given time. Please check again later." % status) | ||
|
||
def add_schema_metadata(self, site, schema, metadata, filename, fp, mimetype="application/x-parquet"): | ||
""" | ||
Create a new Source from a parquet file using schema and metadata. | ||
|
||
Parameters: | ||
site (shoji.Catalog): a shoji Catalog object, from which we acquire session and sources url | ||
schema (dict): json string containing schema | ||
metadata (dict): json string containing metadata | ||
filename (str): name of file being uploaded | ||
fp (BufferedReader): opened file object | ||
mimetype (str): mimetype of file being uploaded | ||
|
||
Returns: | ||
shoji.Entity: Shoji entity containing the source url | ||
""" | ||
response = site.session.post( | ||
site.catalogs.sources, | ||
files={ | ||
"uploaded_file": (filename, fp, mimetype) | ||
}, | ||
data={ | ||
"schema": json.dumps(schema), | ||
"metadata": json.dumps(metadata), | ||
"crunchlake": "create", | ||
"dataset_id": "None" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no dataset ID to create a new source. Sources are not linked to datasets. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The argument is required for the sources post request endpoint. Otherwise there's errors. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then it looks like that is incorrect. Sources are not linked to datasets. This controller should not create sources depending on datasets. The relationship is the other way around. Sources do not point to datasets. Datasets point to sources. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm simply making a post request with the help of pycrunch to the endpoint that requires a
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think this is a bug that has been introduced on the backend. And we should not grow this bug. Sources do not hold references to Datasets. Because multiple datasets are allowed to source rows from the same source entity. In which case, which of those multiple datasets does @mgdotdev Why does the CrunchLake source require to know about a Dataset ID? For a fresh user, they will first have to upload a parquet file while they do not have any Datasets existing in the system yet. And only later they can create a Dataset by using this source. This relation is backwards. |
||
} | ||
) | ||
return shoji.Entity(self=response.headers.get("Location"), session=site.session) | ||
|
||
def add_source(self, ds, filename, fp, mimetype): | ||
"""Create a new Source on the given dataset and return its URL.""" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this argument?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's crunchlake specific. @mgdotdev can expand on this.