Add enabled as a source config #5008

emmyoop · 2022-04-07T16:35:13Z

resolves #3662

Description

This only adds enabled as a config. Tests have been slimmed down to show what is expected to work. Since you can't set enabled as a property currently, we don't need to deal with overrides. enabled as a config is also highly valuable to the users of dbt.

The rest of the configs will be added in #5003.

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 added information about my change to be included in the CHANGELOG.

github-actions · 2022-04-07T16:35:30Z

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

jtcohen6

Whoops, I left my comments in #5003 instead of here. Some of the context there may still be helpful/interesting to you, specifically the piece about unrendered_config.

In any case, I think you're on the right track here! Looks good in local testing. I'd be very happy about shipping this as a standalone piece of net-new functionality in v1.1.

jtcohen6 · 2022-04-08T14:57:35Z

core/dbt/parser/sources.py

+        # configs with precendence set
+        precedence_configs = dict()
+        # first apply source configs
+        precedence_configs.update(target.source.config)
+        # then overrite anything that is defined on source tables
+        precedence_configs.update(target.table.config)


Yep, nice! This solves for #5003 (comment)

The logic here might get trickier when trying to account for configs that can also be defined as top-level node keys. For just enabled, though, this is perfect

yeah this is the part that I really wasn't sure was a good idea. Is there a merge_config_dict function of sorts that will handle some of this?

added a comment to this effect at the very least.

jtcohen6 · 2022-04-08T14:57:51Z

core/dbt/contracts/graph/model_config.py

@@ -335,6 +335,40 @@ def replace(self, **kwargs):
 @dataclass
 class SourceConfig(BaseConfig):
    enabled: bool = True
+    # to be implmented to complete CT-201


nathaniel-may · 2022-04-08T15:42:26Z

blocked on #5015

Co-authored-by: Jeremy Cohen <[email protected]>

nathaniel-may · 2022-04-08T18:04:21Z

looking into the one remaining failure.
failing sql value: '\ncreate schema if not exists test16494413810009909306_test_permissions'

emmyoop · 2022-04-11T14:31:32Z

core/dbt/parser/sources.py

+        # this is not quite complex enough for configs that can be set as top-level node keys, but
+        # it works while source configs can only include `enabled`.


This is so useful for future us.

In the other nodes configs are merged by the code in the BaseConfig object, in the 'update_from' method, which handles merging things like tags and meta, etc. (MergeBehavior, etc). Which is called by 'calculate_node_config'. I imagine you'll need to use some of that code to handle merging the Source configs with the SourceTable configs?

emmyoop

This looks good to me as a first step to the end goal. I'm not going to approve it since I wrote some of it too! I think it would be great if @gshank could take a look.

gshank

Looks good. I left a few comments. The merging of configs will have to be handled eventually but that can happen in the next phase.

core/dbt/contracts/graph/unparsed.py

gshank · 2022-04-11T16:42:12Z

core/dbt/parser/sources.py

+        # this is not quite complex enough for configs that can be set as top-level node keys, but
+        # it works while source configs can only include `enabled`.


In the other nodes configs are merged by the code in the BaseConfig object, in the 'update_from' method, which handles merging things like tags and meta, etc. (MergeBehavior, etc). Which is called by 'calculate_node_config'. I imagine you'll need to use some of that code to handle merging the Source configs with the SourceTable configs?

nathaniel-may · 2022-04-11T19:50:01Z

Also not approving since I wrote some of this as well. Thanks for the review, @gshank!

* initial pass at source config test w/o overrides * Update tests/functional/sources/test_source_configs.py Co-authored-by: Jeremy Cohen <[email protected]> * Update tests/functional/sources/test_source_configs.py Co-authored-by: Jeremy Cohen <[email protected]> * tweaks from feedback * clean up some test logic - add override tests * add new fields to source config class * fix odd formatting * got a test working * removed unused tests * removed extra fields from SourceConfig class * fixed next failing unit test * adding back missing import * first pass at adding source table configs * updated remaining tests to pass * remove source override tests * add comment for config merging * changelog * remove old comments * hacky fix for permission test * remove unhelpful test * adding back test file that was accidentally deleted Co-authored-by: Jeremy Cohen <[email protected]> Co-authored-by: Nathaniel May <[email protected]> Co-authored-by: Chenyu Li <[email protected]>

cla-bot bot added the cla:yes label Apr 7, 2022

emmyoop changed the base branch from main to feature/ct-201-source-config April 7, 2022 18:15

emmyoop closed this Apr 7, 2022

emmyoop reopened this Apr 7, 2022

emmyoop added the Skip Changelog Skips GHA to check for changelog file label Apr 7, 2022

emmyoop changed the base branch from feature/ct-201-source-config to main April 7, 2022 18:27

emmyoop force-pushed the er/slim-down-requirements branch from d0210ec to edd270b Compare April 7, 2022 18:30

jtcohen6 reviewed Apr 8, 2022

View reviewed changes

nathaniel-may mentioned this pull request Apr 8, 2022

[Feature] CT-201 source config WIP #5003

Closed

4 tasks

nathaniel-may changed the title ~~[WIP] First pass at adding enabled as a config~~ Add enabled as a source config Apr 8, 2022

emmyoop and others added 16 commits April 8, 2022 13:21

initial pass at source config test w/o overrides

8145832

Update tests/functional/sources/test_source_configs.py

c22b424

Co-authored-by: Jeremy Cohen <[email protected]>

Update tests/functional/sources/test_source_configs.py

8f71d29

Co-authored-by: Jeremy Cohen <[email protected]>

tweaks from feedback

ca01d5c

clean up some test logic - add override tests

1d5ca06

add new fields to source config class

d6aa961

fix odd formatting

60561f9

got a test working

8bc2d40

removed unused tests

872dae2

removed extra fields from SourceConfig class

dda443d

fixed next failing unit test

7dfd836

adding back missing import

05448ab

first pass at adding source table configs

3507cf0

updated remaining tests to pass

bef9eb2

remove source override tests

b172fef

add comment for config merging

08d70cf

nathaniel-may force-pushed the er/slim-down-requirements branch from 313a3ad to 08d70cf Compare April 8, 2022 17:21

nathaniel-may marked this pull request as ready for review April 8, 2022 17:22

nathaniel-may requested review from a team as code owners April 8, 2022 17:22

nathaniel-may requested review from gshank and nathaniel-may April 8, 2022 17:22

changelog

5c515d9

Nathaniel May and others added 5 commits April 8, 2022 14:49

remove old comments

612299d

hacky fix for permission test

f3ddf3b

Merge branch 'main' into er/slim-down-requirements

497a2fa

remove unhelpful test

2042262

adding back test file that was accidentally deleted

582be80

emmyoop removed the Skip Changelog Skips GHA to check for changelog file label Apr 11, 2022

emmyoop commented Apr 11, 2022

View reviewed changes

jtcohen6 mentioned this pull request Apr 11, 2022

(#4884) Add support for ratio metrics #5027

Merged

14 tasks

gshank approved these changes Apr 11, 2022

View reviewed changes

nathaniel-may mentioned this pull request Apr 11, 2022

[CT-201] Reconcile configs + properties for sources #3662

Closed

emmyoop merged commit cce8fda into main Apr 12, 2022

emmyoop deleted the er/slim-down-requirements branch April 12, 2022 15:27

jtcohen6 mentioned this pull request Apr 13, 2022

Sources can configure enabled inline dbt-labs/docs.getdbt.com#1331

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add enabled as a source config #5008

Add enabled as a source config #5008

emmyoop commented Apr 7, 2022 •

edited by nathaniel-may

Loading

github-actions bot commented Apr 7, 2022

jtcohen6 left a comment

jtcohen6 Apr 8, 2022

nathaniel-may Apr 8, 2022

nathaniel-may Apr 8, 2022

jtcohen6 Apr 8, 2022

nathaniel-may commented Apr 8, 2022

nathaniel-may commented Apr 8, 2022 •

edited

Loading

emmyoop Apr 11, 2022

gshank Apr 11, 2022

emmyoop left a comment

gshank left a comment

gshank Apr 11, 2022

nathaniel-may commented Apr 11, 2022

		# this is not quite complex enough for configs that can be set as top-level node keys, but
		# it works while source configs can only include `enabled`.

Add enabled as a source config #5008

Add enabled as a source config #5008

Conversation

emmyoop commented Apr 7, 2022 • edited by nathaniel-may Loading

Description

Checklist

github-actions bot commented Apr 7, 2022

jtcohen6 left a comment

Choose a reason for hiding this comment

jtcohen6 Apr 8, 2022

Choose a reason for hiding this comment

nathaniel-may Apr 8, 2022

Choose a reason for hiding this comment

nathaniel-may Apr 8, 2022

Choose a reason for hiding this comment

jtcohen6 Apr 8, 2022

Choose a reason for hiding this comment

nathaniel-may commented Apr 8, 2022

nathaniel-may commented Apr 8, 2022 • edited Loading

emmyoop Apr 11, 2022

Choose a reason for hiding this comment

gshank Apr 11, 2022

Choose a reason for hiding this comment

emmyoop left a comment

Choose a reason for hiding this comment

gshank left a comment

Choose a reason for hiding this comment

gshank Apr 11, 2022

Choose a reason for hiding this comment

nathaniel-may commented Apr 11, 2022

emmyoop commented Apr 7, 2022 •

edited by nathaniel-may

Loading

nathaniel-may commented Apr 8, 2022 •

edited

Loading