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.

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

[CT-3401] [Bug] dbt retry fails to apply --full-refresh to downstream models and vars #9112

Closed
2 tasks done
lennons301 opened this issue Nov 17, 2023 · 6 comments
Closed
2 tasks done
Assignees
Labels
backport 1.7.latest bug Something isn't working High Severity bug with significant impact that should be resolved in a reasonable timeframe Impact: Orch retry

Comments

@lennons301
Copy link

lennons301 commented Nov 17, 2023

Is this a new bug in dbt-core?

  • I believe this is a new bug in dbt-core
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

dbt retry fails to pass --full-refresh flag from the original invocation to all models in the retry DAG after the first one

Haven't used dbt retry in earlier versions so unable to say if this is a regression or not

Expected Behavior

dbt retry creates a new DAG starting from the point of failure, continuing with everything downstream

All flags/args passed in the previous invocation should be applied to all models in the retry DAG

Steps To Reproduce

  1. Create 2 incremental models. The second model should reference the first. In both models log the value of is_incremental with {{ log('is_incremental is' ~ is_incremental(), info=True)}}

  2. Create a compilation error in model 1 and invoke dbt run --select +model_2 --full-refresh

This should set is_incremental() to False and

  • produce a compilation error in model 1
  • skip model 2
  1. Correct your compilation error in model 1 and invoke dbt retry

Model one will now run, logging is_incremental = False (expected behaviour)
Model two will now run, logging is_incremental = True (not desired behaviour)

Relevant log output

'''
dbt retry
10:49:37  Running with dbt=1.6.0
10:49:37  Registered adapter: trino=1.6.0
10:49:37  is_incremental isFalse
10:49:37  Found 2 models
10:49:37  
10:49:38  Concurrency: 1 threads (target='local')
10:49:38  
10:49:38  1 of 2 START sql incremental model model_1  [RUN]
10:49:38  is_incremental isFalse
10:49:41  1 of 2 OK created sql incremental model model_1 [SUCCESS in 2.77s]
10:49:41  2 of 2 START sql incremental model model_2  [RUN]
10:49:41  is_incremental isTrue
'''

Environment

- OS: Ubuntu 20.04.6
- Python:3.10.13
- dbt:1.6.0

Which database adapter are you using with dbt?

other (mention it in "Additional Context")

Additional Context

Using dbt-trino 1.6.0

@lennons301 lennons301 added bug Something isn't working triage labels Nov 17, 2023
@github-actions github-actions bot changed the title [Bug] dbt retry fails to apply --full-refresh to downstream models [CT-3401] [Bug] dbt retry fails to apply --full-refresh to downstream models Nov 17, 2023
@lennons301
Copy link
Author

This also seems to apply to vars passed in with --vars '{var_name: var_value}' - except the vars appear not be passed to the retry DAG at all, wheras --full-refresh is applied to the first task in a retry DAG, but not to everything downstream

@graciegoheen graciegoheen changed the title [CT-3401] [Bug] dbt retry fails to apply --full-refresh to downstream models [CT-3401] [Bug] dbt retry fails to apply --full-refresh to downstream models and vars Nov 20, 2023
@graciegoheen graciegoheen added High Severity bug with significant impact that should be resolved in a reasonable timeframe backport 1.7.latest backport 1.6.latest and removed triage labels Nov 20, 2023
@graciegoheen
Copy link
Contributor

graciegoheen commented Nov 20, 2023

Notes from refinement:

@jtcohen6
Copy link
Contributor

Silly repro case:

select {{ var('something') }} as {{ flags.FULL_REFRESH }}
$ dbt run --full-refresh --vars 'something: else'
$ mkdir state
$ mv target/run_results.json state/
$ dbt retry --state state/

After a bit of poking, I'm seeing the issue crop up here, when we initialize Flags from the dictionary of combined_args:

combined_args = {**previous_args, **current_args}
retry_flags = Flags.from_dict(cli_command, combined_args)

ipdb> combined_args['full_refresh']
True
ipdb> combined_args['vars']
{'something': 'else'}
ipdb> cli_command
<Command.RUN: 'run'>
ipdb> retry_flags
Flags()
ipdb> retry_flags.full_refresh
False
ipdb> retry_flags.vars
{'something': 'else'}

The combined_args dict has the appropriate values of --full-refresh and --vars. The retry_flags dict overwrites the value of full_refresh with the default. Even though the value of vars looks correct, we're not appropriately patching cli_vars somewhere, so we still encounter this runtime error:

Compilation Error in model my_model (models/my_model.sql)
  Required var 'something' not found in config:
  Vars supplied to my_model = {}

@aranke
Copy link
Member

aranke commented Nov 21, 2023

Discussed offline with @jtcohen6, comment doesn't apply but leaving for posterity.

Here's the list where we can add in --full-refresh to resolve this issue:

OVERRIDE_PARENT_FLAGS = {
"log_path",
"output_path",
"profiles_dir",
"profiles_dir_exists_false",
"project_dir",
"defer_state",
"deprecated_state",
"target_path",
}

@graciegoheen
Copy link
Contributor

@aranke In case you didn't see the notes from refinement - "We decided that you should be able to override the path configs the the retry command. For every other flag though, whatever was the setting for the original invocation, use the same flags for retry."

This issue is about both full_refresh and vars

@jtcohen6
Copy link
Contributor

From live discussion: Let's investigate the fix, and backport to v1.7 only. Rationale: v1.6 is now under "critical" support, and it is very much still usable, just with this known bug. Users will be able to upgrade to v1.7 for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.7.latest bug Something isn't working High Severity bug with significant impact that should be resolved in a reasonable timeframe Impact: Orch retry
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants