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

fix inplace mutation #842

Closed
wants to merge 1 commit into from

Conversation

dwreeves
Copy link
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
Copy link

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
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
Collaborator Author

dwreeves commented Feb 9, 2024

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

@dwreeves dwreeves closed this Feb 9, 2024
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.

None yet

2 participants