feature/favor-state-node #5859

josephberni · 2022-09-16T13:25:09Z

resolves #5016

Description

Add --favor-state option which enables --defer to favor using --state node even if node exists in current target.

This optionally removes the below second --defer criterion when using the --favor-state flag.

This branch has been created and rebased against an older branch so that the owner of the branch has signed the CLA.

Checklist

I have read the contributing guide and understand what's expected of me
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 opened an issue to add/update docs, or docs changes are not required/relevant for this PR
I have run changie new to create a changelog entry

jtcohen6

Thanks for going through the trouble of a rebase / new branch @josephberni !

jtcohen6 · 2022-09-16T14:25:29Z

core/dbt/main.py

+def _add_favor_state_argument(*subparsers):
+    for sub in subparsers:
+        sub.add_optional_argument_inverse(
+            "--favor-state",


@iknox-fa Do you have a sense of how we want to handle new flag additions, while we're migrating from old to new CLI? Since the new CLI is checked into main, we could ask folks to add it here — but we'll probably also need to check for parity again before formally cutting over in a few months

Sorry I missed this the other day @jtcohen6. I don't have a strong feeling about this-- if we add this flag to the new CLI in this PR I'd be thrilled, but ultimately the "last stop" for this would be when we implement the Click version of the run task (#5551).

.changes/unreleased/Features-20220408-165459.yaml

josephberni · 2022-09-20T10:38:18Z

Will squash all commits once all comments have been resolved :)

gshank · 2022-09-23T23:06:11Z

Why are you overriding '_get_deferred_manifest' and 'defer_to_manifest' in dbt.task.run? Those methods were in the compile task because the selectors apply to a lot of commands besides run.

josephberni · 2022-09-30T09:29:26Z

Why are you overriding '_get_deferred_manifest' and 'defer_to_manifest' in dbt.task.run? Those methods were in the compile task because the selectors apply to a lot of commands besides run.

I've removed these functions from the dbt.task.run file as they exist in the compile file. Thanks for the spot.

josephberni · 2022-10-19T13:22:33Z

Do I need to do anything on this @jtcohen6, appreciate Coalesce likely has you all tied up but want to make sure you are not waiting on me?

stu-k

LGTM 👍

stu-k · 2022-11-03T20:52:33Z

Closing and re-opening to trigger github actions which seem to have stalled.

core/dbt/contracts/graph/manifest.py

Co-authored-by: Stu Kilgore <stuart.kilgore@gmail.com>

stu-k

Ah, I didn't catch these. Since introducing python models the compiled_sql attribute on nodes has been renamed to compiled_code!

test/integration/062_defer_state_tests/test_defer_state.py

Co-authored-by: Stu Kilgore <stuart.kilgore@gmail.com>

josephberni · 2022-11-07T15:29:33Z

Thanks @stu-k ! Once the tests pass I will squash the commits

stu-k · 2022-11-07T15:30:15Z

@josephberni no problem, thank you for taking this on!

stu-k · 2022-11-07T15:39:07Z

test/integration/062_defer_state_tests/test_defer_state.py

+        assert len(results) == 2
+
+        # because the seed exists in other schema, we should defer it
+        assert self.other_schema not in results[0].node.compiled_code


Looks like this is the line causing the tests to fail 🤔

I was creating the seed in the other_schema and checking it wasn't there, resolved this. If all OK will squash commits.

test/integration/062_defer_state_tests/test_defer_state.py

stu-k

Great work! Thank you for putting this together!

josephberni requested a review from a team September 16, 2022 13:25

josephberni requested review from a team as code owners September 16, 2022 13:25

josephberni requested review from iknox-fa and gshank September 16, 2022 13:25

cla-bot bot added the cla:yes label Sep 16, 2022

josephberni mentioned this pull request Sep 16, 2022

add favor-state flag #5017

Closed

4 tasks

jtcohen6 reviewed Sep 16, 2022

View reviewed changes

jtcohen6 added Team:Execution ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering labels Sep 16, 2022

jtcohen6 added the cli label Sep 20, 2022

josephberni force-pushed the feature/favor-state-node branch from 7b95d99 to 5785a81 Compare September 30, 2022 09:26

favor-state-node

a3726d5

josephberni force-pushed the feature/favor-state-node branch from 5785a81 to a3726d5 Compare September 30, 2022 09:27

stu-k approved these changes Nov 3, 2022

View reviewed changes

stu-k closed this Nov 3, 2022

stu-k reopened this Nov 3, 2022

stu-k suggested changes Nov 4, 2022

View reviewed changes

core/dbt/contracts/graph/manifest.py Outdated Show resolved Hide resolved

Update core/dbt/contracts/graph/manifest.py

2ab0291

Co-authored-by: Stu Kilgore <stuart.kilgore@gmail.com>

stu-k suggested changes Nov 7, 2022

View reviewed changes

test/integration/062_defer_state_tests/test_defer_state.py Outdated Show resolved Hide resolved

test/integration/062_defer_state_tests/test_defer_state.py Outdated Show resolved Hide resolved

test/integration/062_defer_state_tests/test_defer_state.py Outdated Show resolved Hide resolved

josephberni and others added 3 commits November 7, 2022 15:28

Update test/integration/062_defer_state_tests/test_defer_state.py

4b6d910

Co-authored-by: Stu Kilgore <stuart.kilgore@gmail.com>

Update test/integration/062_defer_state_tests/test_defer_state.py

a5e77ff

Co-authored-by: Stu Kilgore <stuart.kilgore@gmail.com>

Update test/integration/062_defer_state_tests/test_defer_state.py

166fb1d

Co-authored-by: Stu Kilgore <stuart.kilgore@gmail.com>

stu-k reviewed Nov 7, 2022

View reviewed changes

josephberni commented Nov 9, 2022

View reviewed changes

test/integration/062_defer_state_tests/test_defer_state.py Outdated Show resolved Hide resolved

Update test/integration/062_defer_state_tests/test_defer_state.py

e428cd0

stu-k approved these changes Nov 9, 2022

View reviewed changes

stu-k merged commit 73116fb into dbt-labs:main Nov 9, 2022

jtcohen6 mentioned this pull request Nov 21, 2022

[CT-927] dbt run works in click #5551

Closed

NiallRees mentioned this pull request Dec 6, 2022

DBT_FAVOR_STATE_STATE -> DBT_FAVOR_STATE #6392

Merged

6 tasks

jtcohen6 mentioned this pull request Jan 2, 2023

Fix issue link for changelog entry from PR#5859 #6496

Merged

This was referenced Jan 11, 2023

[CT-901] [Epic] API-ification + CLI - Phase 1 #5527

Closed

[CT-1804] Add --favor-state as param in click CLI #6585

Closed

convert 062_defer_state_tests #6616

Merged

[CT-1823] [Bug] [1.4.0rc1] --favor-state not working as intended #6617

Closed

iknox-fa mentioned this pull request Jan 30, 2023

testing merge strat - do not merge ns/add to committer list #6783

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature/favor-state-node #5859

feature/favor-state-node #5859

josephberni commented Sep 16, 2022 •

edited

Loading

jtcohen6 left a comment

jtcohen6 Sep 16, 2022

iknox-fa Sep 27, 2022 •

edited

Loading

josephberni commented Sep 20, 2022

gshank commented Sep 23, 2022 •

edited

Loading

josephberni commented Sep 30, 2022

josephberni commented Oct 19, 2022

stu-k left a comment

stu-k commented Nov 3, 2022

stu-k left a comment

josephberni commented Nov 7, 2022

stu-k commented Nov 7, 2022

stu-k Nov 7, 2022

josephberni Nov 9, 2022

stu-k left a comment

feature/favor-state-node #5859

feature/favor-state-node #5859

Conversation

josephberni commented Sep 16, 2022 • edited Loading

Description

Checklist

jtcohen6 left a comment

Choose a reason for hiding this comment

jtcohen6 Sep 16, 2022

Choose a reason for hiding this comment

iknox-fa Sep 27, 2022 • edited Loading

Choose a reason for hiding this comment

josephberni commented Sep 20, 2022

gshank commented Sep 23, 2022 • edited Loading

josephberni commented Sep 30, 2022

josephberni commented Oct 19, 2022

stu-k left a comment

Choose a reason for hiding this comment

stu-k commented Nov 3, 2022

stu-k left a comment

Choose a reason for hiding this comment

josephberni commented Nov 7, 2022

stu-k commented Nov 7, 2022

stu-k Nov 7, 2022

Choose a reason for hiding this comment

josephberni Nov 9, 2022

Choose a reason for hiding this comment

stu-k left a comment

Choose a reason for hiding this comment

josephberni commented Sep 16, 2022 •

edited

Loading

iknox-fa Sep 27, 2022 •

edited

Loading

gshank commented Sep 23, 2022 •

edited

Loading