Allow setting access in config in addition to properties #8635
Conversation
gshank
requested review from
jzhu13 and
MichelleArk
and removed request for
a team
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #8635 +/- ##
==========================================
- Coverage 86.61% 86.60% -0.01%
==========================================
Files 174 175 +1
Lines 25590 25607 +17
==========================================
+ Hits 22164 22177 +13
- Misses 3426 3430 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
core/dbt/parser/base.py
Outdated
| and "access" in config_dict | ||
| and config_dict["access"] |
There was a problem hiding this comment.
Nit: simplify to `config_dict.get('access', None)
| and "access" in config_dict | ||
| and config_dict["access"] | ||
| ): | ||
| if AccessType.is_valid(config_dict["access"]): |
There was a problem hiding this comment.
Do we need nested if statements here, or can we combine into 1 level?
There was a problem hiding this comment.
They can't be combined, the logic would be wrong.
| manifest = run_dbt(["parse"]) | ||
| # ['model.test.another_model', 'model.test.my_model', 'model.test.ref_my_model', 'model.test.people_model', 'model.test.metricflow_time_spine'] | ||
| assert len(manifest.nodes) == 5 | ||
| assert manifest.nodes["model.test.my_model"].access == AccessType.Private |
There was a problem hiding this comment.
Nit: can we parameterize this test to avoid repeating code?
There was a problem hiding this comment.
Not really. It's not doing the same test multiple times with different inputs, it's checking multiple things in the output of one test.
| @@ -0,0 +1,6 @@ | |||
| kind: Features | |||
| body: Allow setting access in config | |||
There was a problem hiding this comment.
Can you make this a bit more clear so that someone reading it without context will have a better idea what it means?
|
Opened a new issue in dbt-labs/docs.getdbt.com: dbt-labs/docs.getdbt.com#4075 |
resolves #8383
Problem
Setting "access" had been limited to setting as a node property, but users want to be able to set it in dbt_project.yml
Solution
Create a ModelConfig config with an "access" field. Wire up all of the pieces to allow it to be properly parsed and promoted to a node property.
Checklist