Skip to content
This repository was archived by the owner on Feb 23, 2022. It is now read-only.

Introduce Table Metadata#484

Merged
jjnesbitt merged 9 commits intomasterfrom
table-graph-metadata
Nov 16, 2020
Merged

Introduce Table Metadata#484
jjnesbitt merged 9 commits intomasterfrom
table-graph-metadata

Conversation

@jjnesbitt
Copy link
Copy Markdown
Member

Closes #482

This is a basic first approach to table metadata. In this approach, table metadata is passed as a query string argument to the CSV upload endpoint. This is not ideal, and is only in place until this issue is addressed.

One of the side effects is that this PR slightly changes how tables are created. Now, the entire workspace instance is passed into the Table constructor, which the constructor selects things from. This was mainly needed for referring to the workspace metadata collection. I actually think this approach is cleaner overall.

@jjnesbitt jjnesbitt requested review from JackWilb and waxlamp November 6, 2020 18:24
@waxlamp waxlamp temporarily deployed to multinet-pr-484 November 6, 2020 18:26 Inactive
Copy link
Copy Markdown
Contributor

@waxlamp waxlamp left a comment

Choose a reason for hiding this comment

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

This looks awesome. Thanks for being so fast. I have a few requested changes. If you'd prefer me to submit those changes for a counterreview by you (in the interest of time, mainly) please let me know.

Comment thread multinet/types.py Outdated
Comment thread multinet/api.py Outdated
Comment thread multinet/db/models/table.py
Comment thread multinet/db/models/table.py Outdated
Comment thread multinet/swagger/set_metadata.yaml Outdated
Comment thread multinet/uploaders/csv.py Outdated
Comment thread multinet/uploaders/swagger/csv.yaml
Copy link
Copy Markdown
Member

@JackWilb JackWilb left a comment

Choose a reason for hiding this comment

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

Here are a couple of questions and a couple of copy-paste problems I found

Comment thread multinet/api.py Outdated
Comment thread multinet/api.py
Comment thread multinet/db/models/table.py Outdated
Comment thread multinet/swagger/get_metadata.yaml Outdated
Comment thread multinet/swagger/set_metadata.yaml
Comment thread multinet/swagger/set_metadata.yaml Outdated
Comment thread multinet/uploaders/csv.py Outdated
@waxlamp waxlamp temporarily deployed to multinet-pr-484 November 9, 2020 17:56 Inactive
This also updates the BadQueryArgument error, as it previously included
an "allowed" field, which may not always apply.
@jjnesbitt jjnesbitt temporarily deployed to multinet-pr-484 November 9, 2020 18:24 Inactive
@jjnesbitt jjnesbitt temporarily deployed to multinet-pr-484 November 9, 2020 18:27 Inactive
@jjnesbitt jjnesbitt requested review from JackWilb and waxlamp November 9, 2020 18:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement table metadata

3 participants