Skip to content

fix inplace mutation#842

Closed
dwreeves wants to merge 1 commit into
astronomer:mainfrom
dwreeves:fix-inplace-mutation
Closed

fix inplace mutation#842
dwreeves wants to merge 1 commit into
astronomer:mainfrom
dwreeves:fix-inplace-mutation

Conversation

@dwreeves
Copy link
Copy Markdown
Collaborator

@dwreeves dwreeves commented Feb 9, 2024

operator_args.pop() causes an in-place mutation, which long story short caused an issue for me today. I think it is a lot safer to rely on a copy() if the DbtToAirflowConverter is going to mutate the user inputs.

@dwreeves dwreeves requested a review from a team as a code owner February 9, 2024 19:11
@dosubot dosubot Bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Feb 9, 2024
@netlify
Copy link
Copy Markdown

netlify Bot commented Feb 9, 2024

Deploy Preview for sunny-pastelito-5ecb04 canceled.

Name Link
🔨 Latest commit 5c6210d
🔍 Latest deploy log https://app.netlify.com/sites/sunny-pastelito-5ecb04/deploys/65c678feeea5cd0008576275

@dosubot dosubot Bot added area:parsing Related to parsing DAG/DBT improvement, issues, or fixes dbt:run Primarily related to dbt run command or functionality execution:local Related to Local execution environment parsing:custom Related to custom parsing, like custom DAG parsing, custom DBT parsing, etc labels Feb 9, 2024
@jbandoro
Copy link
Copy Markdown
Collaborator

jbandoro commented Feb 9, 2024

Thanks @dwreeves! This fixes the same issue that #835 tries to fix from user reports in Slack. The inplace updates for operator_args were for "env" or "vars" which #835 fixes, and there shouldn't be any updates in the rest of the converter because they're shallow copied below to the task_args:

task_args = {
**operator_args,
"project_dir": execution_config.project_path,
"profile_config": profile_config,
"emit_datasets": render_config.emit_datasets,
"env": env_vars,
"vars": dbt_vars,
}

@dwreeves
Copy link
Copy Markdown
Collaborator Author

dwreeves commented Feb 9, 2024

Ah I should have looked at existing PRs. I can close this then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:parsing Related to parsing DAG/DBT improvement, issues, or fixes dbt:run Primarily related to dbt run command or functionality execution:local Related to Local execution environment parsing:custom Related to custom parsing, like custom DAG parsing, custom DBT parsing, etc size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants