-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
feat(SIP-95): catalogs in SQL Lab and datasets #28376
Conversation
b8fc10d
to
073767f
Compare
aa3182e
to
65689a6
Compare
65689a6
to
0075484
Compare
@@ -410,6 +412,8 @@ export default function sqlLabReducer(state = {}, action) { | |||
return state; | |||
}, | |||
[actions.LOAD_QUERY_EDITOR]() { | |||
console.log(state); | |||
console.log(state.unsavedQueryEditor); |
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.
console.log(state.unsavedQueryEditor); |
} | ||
|
||
if (isFetched) { | ||
addSuccessToast('List refreshed'); |
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.
translate?
name="select-catalog" | ||
notFoundContent={t('No compatible catalog found')} | ||
placeholder={t('Select catalog or type to search catalogs')} | ||
onChange={item => changeCatalog(item as CatalogOption)} |
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.
did passing just changeCatalog
to the prop fail because of a ts error? (as opposed to the arrow function)
function renderSchemaSelect() { | ||
const refreshIcon = !readOnly && ( | ||
<RefreshLabel | ||
onClick={() => refetch()} | ||
onClick={() => refetchSchemas()} |
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.
onClick={() => refetchSchemas()} | |
onClick={refetchSchemas} |
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.
lgtm.. just some small nits.
SUMMARY
This PR finishes the initial work for #22862, introducing a new hierarchical data level: catalogs.
A few important concepts:
See the option at the bottom of "Advanced" -> "Other" -> "Allow changing catalogs":
Once the feature is enabled, a dropdown will show up the in
DatabaseSelector
component, allowing users to select a catalog. This includes SQL Lab, the page for adding datasets, and the dataset editor.The dropdown is hidden when a database without the feature is selected:
Screen.Recording.2024-05-07.at.5.01.08.PM.mov
Nothing should change for databases that don't support catalogs, or that have it disabled. Relevant APIs have been updated to accept an optional
catalog
. Databases without support will simply ignore it, and databases that support it will use it to establish the connection, or fallback to the default catalog if anull
is passed.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
See above.
TESTING INSTRUCTIONS
WIP, I'm currently updating existing tests and adding new tests.
ADDITIONAL INFORMATION