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.

Sign up for GitHub

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

Jump to bottom

Grant Tests Implementation #62

Merged
merged 6 commits into from
Nov 9, 2022
Merged

Grant Tests Implementation #62

merged 6 commits into from
Nov 9, 2022

Conversation

ravjotbrar
Copy link
Contributor

@ravjotbrar ravjotbrar commented Nov 9, 2022 •
edited
Loading

Summary

This PR makes the necessary changes to support grants in our adapter. All tests provided by dbt-core pass (with modifications). For grants to work, changes need to be made to the relevant "grant" macros. Also made a few miscellaneous changes including updating smoke_tests to not wipe out current profiles.yml file.

Description

Macros

  • support_multiple_grantees_per_dcl_statement: Dremio only supports one grantee per statement
  • get_show_grant_sql: Gets data from privileges table
  • get_grant_sql : Contains sql for granting a privilege
  • get_revoke_sql: Contains sql for revoking a privilege
  • call_dcl_statements: Dremio only supports calling one statement at a time

These macros are all called by the default apply_grants macro provided by dbt-core. For this reason, we need to make sure apply_grants is called in each of the materializations (table, view, seed, incremental).

Tests

  • test_model_grants: Tests grants for views and tables
  • test_incremental_grants: Tests grants for incremental materializations
  • test_seed_grants: Tests grants for seeds
  • test_snapshot_grants: Tests grants for snapshots
  • test_invalid_grants: Tests invalid grants by providing a invalid privilege and invalid user

Miscellaneous

  • Added logic to keep current profiles.yml file in smoke_tests
  • Removed the schema.sql macro, as those methods are overridden in impl.py
  • Added logic to drop a table when drop_schema is called (only spaces allow the entire schema to be deleted)
  • Added copyright headers to basic func tests

Related Issue

#42

Additional Reviewers

@ArgusLi @jlarue26

@ravjotbrar ravjotbrar changed the title Grants Tests WIP Grant Tests Implementation Nov 9, 2022
@ravjotbrar ravjotbrar linked an issue Nov 9, 2022 that may be closed by this pull request
dbt-core 1.2 upgrade: Grants #42
Closed
ArgusLi
ArgusLi reviewed Nov 9, 2022
View reviewed changes
dbt/adapters/dremio/connections.py
@@ -195,24 +195,20 @@ def drop_catalog(self, database, schema):
credentials = connection.credentials
api_parameters = connection.handle.get_parameters()

path_list = self._create_path_list(database, schema)
if database != credentials.datalake:
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason for this change?

ArgusLi
ArgusLi reviewed Nov 9, 2022
View reviewed changes
dbt/adapters/dremio/connections.py

delete_catalog(api_parameters, catalog_info["id"], ssl_verify=False)
delete_catalog(api_parameters, catalog_info["id"], ssl_verify=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the change in indentation here?

@ravjotbrar ravjotbrar merged commit b626bb9 into main Nov 9, 2022
@ravjotbrar ravjotbrar deleted the test-grants branch November 9, 2022 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ArgusLi ArgusLi ArgusLi approved these changes

@jlarue26 jlarue26 Awaiting requested review from jlarue26

Assignees

@ravjotbrar ravjotbrar

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

dbt-core 1.2 upgrade: Grants
2 participants
@ravjotbrar @ArgusLi