Skip to content
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: catalog support for Databricks native #28394

Merged
merged 2 commits into from
May 9, 2024
Merged

Conversation

betodealmeida
Copy link
Member

@betodealmeida betodealmeida commented May 9, 2024

SUMMARY

Add support for catalogs in Databricks, and improve the migration that updates permissions and models with the catalog.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen Shot 2024-05-09 at 9 51 57 AM

TESTING INSTRUCTIONS

Before running the upgrade, add a Databricks database, create a dataset, make a chart, run and save a query. They all should have no catalog information:

databricks_catalogs=# SELECT catalog FROM saved_query UNION ALL SELECT catalog FROM query UNION ALL SELECT catalog FROM tab_state UNION ALL SELECT catalog FROM table_schema UNION ALL SELECT catalog FROM tables;
 catalog
---------





(5 rows)

databricks_catalogs=# SELECT schema_perm FROM slices UNION ALL SELECT schema_perm FROM tables;      schema_perm
------------------------
 [Databricks].[default]
 [Databricks].[default]
(2 rows)

databricks_catalogs=# SELECT * FROM ab_view_menu JOIN ab_permission_view ON ab_view_menu.id = view_menu_id JOIN ab_permission ON ab_permission.id = permission_id WHERE ab_permission.name IN ('schema_access', 'catalog_access');
 id  |             name              | id  | permission_id | view_menu_id | id |      name
-----+-------------------------------+-----+---------------+--------------+----+----------------
  76 | [Databricks].[default]        | 167 |            74 |           76 | 74 | schema_access
(1 row)

Run the upgrade, and check that the catalog is present:

databricks_catalogs=# SELECT catalog FROM saved_query UNION ALL SELECT catalog FROM query UNION ALL SELECT catalog FROM tab_state UNION ALL SELECT catalog FROM table_schema UNION ALL SELECT catalog FROM tables;
    catalog
----------------
 hive_metastore
 hive_metastore
 hive_metastore
 hive_metastore
 hive_metastore
(5 rows)

databricks_catalogs=# SELECT schema_perm FROM slices UNION ALL SELECT schema_perm FROM tables;
               schema_perm
-----------------------------------------
 [Databricks].[hive_metastore].[default]
 [Databricks].[hive_metastore].[default]
(2 rows)

databricks_catalogs=# SELECT * FROM ab_view_menu JOIN ab_permission_view ON ab_view_menu.id = view_menu_id JOIN ab_permission ON ab_permission.id = permission_id WHERE ab_permission.name IN ('schema_access', 'catalog_access');
 id  |                  name                   | id  | permission_id | view_menu_id | id |      name
-----+-----------------------------------------+-----+---------------+--------------+----+----------------
  76 | [Databricks].[hive_metastore].[default] | 167 |            74 |           76 | 74 | schema_access
 110 | [Databricks].[hive_metastore]           | 192 |            76 |          110 | 76 | catalog_access
(3 rows)

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@github-actions github-actions bot added the risk:db-migration PRs that require a DB migration label May 9, 2024
@betodealmeida betodealmeida marked this pull request as ready for review May 9, 2024 02:48
@betodealmeida betodealmeida requested a review from a team as a code owner May 9, 2024 02:48
@betodealmeida betodealmeida force-pushed the databricks-catalogs branch 9 times, most recently from b79cb99 to b1d3d4e Compare May 9, 2024 14:00
Copy link
Member

@eschutho eschutho left a comment

Choose a reason for hiding this comment

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

LGTM

@betodealmeida betodealmeida merged commit f29e1e4 into master May 9, 2024
31 checks passed
@rusackas rusackas deleted the databricks-catalogs branch May 9, 2024 22:58
imancrsrk pushed a commit to imancrsrk/superset that referenced this pull request May 10, 2024
jzhao62 pushed a commit to jzhao62/superset that referenced this pull request May 16, 2024
EnxDev pushed a commit to EnxDev/superset that referenced this pull request May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
risk:db-migration PRs that require a DB migration size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants