-
-
Notifications
You must be signed in to change notification settings - Fork 362
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
Implement tables.list
rpc endpoint
#3599
Conversation
tables.list
rpc endpoint
@mathemancer, @seancolsen As of now the |
Good question. I would lean towards keeping it. I don't see much harm in having it there. And the data structures in the front end do actually appear to use that property as returned from the REST API. It's possible it would be easy to refactor that stuff out of the front end. But it would take me some time to investigate deeply enough to form a definitive answer. |
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.
I noticed one detail that we should discuss, but I prefer merging this and then fixing it once we've decided on the convention to use.
schema: The `oid` of the schema where the table lives. | ||
description: The description of the table. | ||
""" | ||
oid: int |
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.
I'm okay with this, but we should chat at some point with @seancolsen about preferences w.r.t. specific ID names instead of just id
in this type of case. I went with id
rather than attnum
for column info, for example. I don't really have a strong preference, but we should choose one way and stick with it.
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.
As I'm looking at this more, I think id
is more consistent with referring to the schema
and using its (o)id.
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.
I added this as a topic for discussion at our technical beta planning meeting.
Fixes #3600
Checklist
Update index.md
).develop
branch of the repositoryvisible errors.
Developer Certificate of Origin
Developer Certificate of Origin