feat: add metastore loader config #1134
Conversation
querybook/server/const/metastore.py
Outdated
|
|
||
|
|
||
| class MetadataMode(Enum): | ||
| READ_ONLY = "read_only" # metadata will be read-only on Querybook UI |
There was a problem hiding this comment.
should this also include option for it will be editable and only saved in qurybook db or that is writeback?
There was a problem hiding this comment.
My thought was: if a metadata type key is not in the loader config, then it will be editable and only saved in querybook db, which is today's behavior.
I can also add another option for it, and make it default
read_only:
write_back: default
write_through: write to querybook db and also write back to metastore
| verify_metastore_permission(metastore_id) | ||
| metastore_loader = get_metastore_loader(metastore_id) | ||
|
|
||
| return metastore_loader.get_metastore_link( |
There was a problem hiding this comment.
need to validate if metastore_loader indeed is in read only mode for that metadata type
There was a problem hiding this comment.
I thought it over, perhaps we dont need to restrict the metastore link only for read-only mode . The metastore link exists no matter it's read-only or not, but as long as there is a link. For now, we only use it for redirection. but maybe we could still put the link some where even when it will write back to the metastore.
querybook/webapp/components/DataTableViewColumn/DataTableColumnCard.tsx
Outdated
Show resolved
Hide resolved
querybook/webapp/components/DataTableViewColumn/DataTableViewColumn.tsx
Outdated
Show resolved
Hide resolved
querybook/server/const/metastore.py
Outdated
| # Metadata will be read-only on Querybook UI and it will redirect to the metastore link for editing. | ||
| READ_ONLY = "read_only" | ||
|
|
||
| # On saving, metadata will only be written back querybook db. This is the default mode if not specified. |
There was a problem hiding this comment.
the default value in MetastoreLoaderConfig is None, can you put some way to associate WRITE_BACK if no value is provided for the entity?
There was a problem hiding this comment.
also WRITE_BACK sounds like it is going to write back to the metastore, maybe WRITE_LOCAL or WRITE_QUERYBOOK_ONLY?
| table_id, | ||
| metadata_type, | ||
| ): | ||
| with DBSession() as session: |
There was a problem hiding this comment.
session is not needed for API, its automatic
querybook/server/const/metastore.py
Outdated
| COLUMN_DESCRIPTION = "column_description" | ||
| OWNER = "owner" | ||
| TAG = "tag" | ||
| DOMAIN = "domain" |
There was a problem hiding this comment.
lets remove DOMAIN in this PR for now, since this metadata entity does not actually exist.
Once we do treat domain as something in querybook, we can add it back in
* feat: add metastore config * comments * rename * remove session
* feat: add metastore config * comments * rename * remove session
As we may sync more metadata types from metastore, like owners, tags, here we are adding a config for the metastore loader so that we can manage whether the metadata should be read-only or written back to metastore.
By default, it will keep the current behavior, which is only writing into querybook db.