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

Add apply_grants call to materialization macros #381

Merged
merged 18 commits into from
Jul 12, 2022
Merged

Add apply_grants call to materialization macros #381

merged 18 commits into from
Jul 12, 2022

Conversation

gshank
Copy link
Contributor

@gshank gshank commented Jun 30, 2022

resolves #366

Description

Add calls to 'apply_grants' to table, snapshot, and incremental materializations.

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt-spark next" section.

@cla-bot cla-bot bot added the cla:yes label Jun 30, 2022
@gshank gshank marked this pull request as draft June 30, 2022 16:55
@gshank
Copy link
Contributor Author

gshank commented Jun 30, 2022

This is not complete. Needs to have create_or_replace_view macro in dbt-core updated. Waiting for adapter zone tests to pull in.

McKnight-42
McKnight-42 approved these changes Jun 30, 2022
View reviewed changes
Copy link
Contributor

@McKnight-42 McKnight-42 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This is looking great.

@gshank
Copy link
Contributor Author

gshank commented Jun 30, 2022

Note: there's not a spark specific seed materialization. There is a materialization/seed.sql, but it has macros like 'load_csv_rows' and 'create_csv_table'.

@jtcohen6
Copy link
Contributor

Thanks for opening the draft PR!

We will likely also need post-processing code to standardize the results returned by show grants, since the column names and contents will be specific to SparkSQL.

I believe that the default get_grant_sql + get_revoke_sql macros will work without changes, but the "adapter zone" tests will tell us for sure.

@dbeatty10 dbeatty10 mentioned this pull request Jul 7, 2022
Closed
9 tasks
@jtcohen6
Copy link
Contributor

jtcohen6 commented Jul 8, 2022 •
edited
Loading

#389 gets this assertion in the dbt-core tests passing, by bringing seeds into line with expected / default behavior: truncate if not --full-refresh, such that grants are carried forward

Update: BigQuery had the same problem, so I added a convenience method in dbt-labs/dbt-core@debc867: seeds_support_partial_refresh(): False. I'm just going to use that here as well, to avoid pulling in the functional change from #389, and keep it in a separate PR / review.

@jtcohen6 jtcohen6 marked this pull request as ready for review July 11, 2022 09:58
@McKnight-42
Copy link
Contributor

changed pointer, to main, and added changelog

@McKnight-42 McKnight-42 requested a review from emmyoop July 12, 2022 05:46
emmyoop
emmyoop approved these changes Jul 12, 2022
View reviewed changes
Copy link
Member

@emmyoop emmyoop left a comment

Choose a reason for hiding this comment

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

LGTM

@McKnight-42 McKnight-42 merged commit 9109fe1 into main Jul 12, 2022
@McKnight-42 McKnight-42 deleted the ct-718-apply_grants branch July 12, 2022 14:14
@ueshin ueshin mentioned this pull request Jul 14, 2022
Merged
ueshin added a commit to databricks/dbt-databricks that referenced this pull request Jul 19, 2022
@ueshin @jtcohen6
@McKnight-42 @gshank
Port: Add apply_grants call to materialization macros (dbt-labs/dbt-s…
e319bbb 
…park#381) (#130)

### Description

Ports in the upstream: Add apply_grants call to materialization macros (dbt-labs/dbt-spark#381).

Co-authored-by: Jeremy Cohen <jeremy@dbtlabs.com>
Co-authored-by: Matthew McKnight <matthew.mcknight@dbtlabs.com>
Co-authored-by: Gerda Shank <gerda@dbtlabs.com>
ueshin added a commit to databricks/dbt-databricks that referenced this pull request Jul 19, 2022
@ueshin @jtcohen6
@McKnight-42 @gshank
Port: Add apply_grants call to materialization macros (dbt-labs/dbt-s…
3aa9d65 
…park#381) (#130)

### Description

Ports in the upstream: Add apply_grants call to materialization macros (dbt-labs/dbt-spark#381).

Co-authored-by: Jeremy Cohen <jeremy@dbtlabs.com>
Co-authored-by: Matthew McKnight <matthew.mcknight@dbtlabs.com>
Co-authored-by: Gerda Shank <gerda@dbtlabs.com>
francescomucio pushed a commit to francescomucio/dbt-spark that referenced this pull request Jul 26, 2022
@gshank @jtcohen6
@McKnight-42 @francescomucio
Add apply_grants call to materialization macros (dbt-labs#381)
f5db7e6 
* Add apply_grants call to materialization macros

* add standardize_grants_dict

* Working grant macros

* Initialize tests in CI

* Refactor to account for core macro changes. Passing tests

* Fix code checks

* Try default__reset_csv_table

* Code checks

* Revert "Try default__reset_csv_table"

This reverts commit 8bd4145.

* Account for refactor in dbt-labs/dbt-core@c763601

* Account for test changes in dbt-labs/dbt-core@debc867

* add changelog

* Empty-Commit

* rerun ci

* rerun ci

* readd a persist_docs call to snapshot.sql

* fix whitespace

Co-authored-by: Jeremy Cohen <jeremy@dbtlabs.com>
Co-authored-by: Matthew McKnight <matthew.mcknight@dbtlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@McKnight-42 McKnight-42 McKnight-42 approved these changes

@emmyoop emmyoop emmyoop approved these changes

@jtcohen6 jtcohen6 Awaiting requested review from jtcohen6

Assignees
No one assigned
Labels
cla:yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CT-718] Add Grants to Materializations
4 participants
@gshank @jtcohen6 @McKnight-42 @emmyoop