Prevent overriding dbt profile fields with profile args of "type" or "method" by jbandoro · Pull Request #702 · astronomer/astronomer-cosmos

jbandoro · 2023-11-23T01:51:12Z

Description

The issue is described in #696 which was discovered when a user was creating a ProfileConfig with GoogleCloudServiceAccountDictProfileMapping(profile_args={"method": "service-account"}) which was overriding the dbt profile method:

astronomer-cosmos/cosmos/profiles/bigquery/service_account_keyfile_dict.py

Line 21 in 24aa38e

dbt_profile_method: str = "service-account-json"

when the profile args are mapped to the created profile below:

astronomer-cosmos/cosmos/profiles/bigquery/service_account_keyfile_dict.py

Lines 42 to 52 in 24aa38e

    
               def profile(self) -> dict[str, Any | None]: 
        
                   """ 
        
                   Generates a GCP profile. 
        
                   Even though the Airflow connection contains hard-coded Service account credentials, 
        
                   we generate a temporary file and the DBT profile uses it. 
        
                   """ 
        
                   return { 
        
                       **self.mapped_params, 
        
                       "threads": 1, 
        
                       **self.profile_args, 
        
                   }

This is not an issue with the profile mapping example above and could happen with any profile mapping by changing the "type" from dbt_profile_type or "method" (if used) from dbt_profile_method in the class.

The fix in this PR is to not allow args with "type" or "method" that are different from the class variables in profile_args.

I think this is better than logging a warning because if either of those fields are different the dbt run with the created profile will fail anyways. This also allows backwards compatibility in the case users have these already set in their profile args and it matches the class variables.

Related Issue(s)

closes #696

Breaking Change?

None

Checklist

I have made corresponding changes to the documentation (if required)
I have added tests that prove my fix is effective or that my feature works

netlify · 2023-11-23T01:51:19Z

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Name	Link
🔨 Latest commit	`ebb5e16`
🔍 Latest deploy log	https://app.netlify.com/sites/amazing-pothos-a3bca0/deploys/655f5a4a5679130008cbafb0

…reserved

tatiana

This looks great, thanks, @jbandoro for fixing this so quickly!

tatiana · 2023-11-23T16:51:48Z

@jbandoro This was released in 1.2.5 🎉

@jbandoro

[Justin Bandoro](https://www.linkedin.com/in/justin-bandoro-592b14a7/) (@jbandoro) is a Data Engineer at Kevala Inc. He's based in San Francisco (USA) and has been an early adopter of Cosmos, using it regularly at his company. Not only has he been using Cosmos since the early stages, but he has consistently improved Cosmos since January 2023: ![Screenshot 2023-12-04 at 16 28 29](https://github.com/astronomer/astronomer-cosmos/assets/272048/43197938-d1ab-431f-b101-b6026e5cd3ab) Some of his contributions include new features, code quality, documentation and overall improvements. Some examples: * Speed up integration tests in 67% #732 * Prevent override of dbt profile fields #702 * Add support for env vars in `RenderConfig` in #690 * Use symbolic links to run local tasks, avoiding to copy potentially huge dbt project folders in #660 * Improve documentation in #638 * Automated and improved the code complexity checks in #629 * Added `DbtDocsGCSOperator` in #616 * Added support for Python 3.7 in #88 and #214 Additionally, he has been interacting with users in the #airflow-dbt Slack channel in a very collaborative and supportive way. We want to promote him as a Cosmos committer and maintainer for all these, recognising his constant efforts and achievements towards our community. Thank you very much, @jbandoro !

…"method" (astronomer#702) The issue is described in astronomer#696 which was discovered when a user was creating a ProfileConfig with `GoogleCloudServiceAccountDictProfileMapping(profile_args={"method": "service-account"})` which was overriding the dbt profile method: https://github.com/astronomer/astronomer-cosmos/blob/24aa38e528e299ef51ca6baf32f5a6185887d432/cosmos/profiles/bigquery/service_account_keyfile_dict.py#L21 when the profile args are mapped to the created profile below: https://github.com/astronomer/astronomer-cosmos/blob/24aa38e528e299ef51ca6baf32f5a6185887d432/cosmos/profiles/bigquery/service_account_keyfile_dict.py#L42-L52 This is not an issue with the profile mapping example above and could happen with any profile mapping by changing the "type" from `dbt_profile_type` or "method" (if used) from `dbt_profile_method` in the class. The fix in this PR is to not allow args with "type" or "method" that are different from the class variables in `profile_args`. I think this is better than logging a warning because if either of those fields are different the dbt run with the created profile will fail anyways. This also allows backwards compatibility in the case users have these already set in their profile args and it matches the class variables. Closes astronomer#696

@binhnq94

Bug fixes * Fix running models that use alias while supporting dbt versions by @binhnq94 in astronomer#662 * Make profiles_yml_path optional for ExecutionMode.DOCKER and KUBERNETES by @MrBones757 in astronomer#681 * Prevent overriding dbt profile fields with profile args of type or method by @jbandoro in astronomer#702 * Fix LoadMode.DBT_LS fail when dbt outputs WarnErrorOptions by @adammarples in astronomer#692 * Add support for env vars in RenderConfig for dbt ls parsing by @jbandoro in astronomer#690 * Add support for Kubernetes on_warning_callback by @david-mag in astronomer#673 * Fix ExecutionConfig.dbt_executable_path to use ``default_factory`` by @jbandoro in astronomer#678 Others * Docs fix: example DAG in the README and docs/index by @tatiana in astronomer#705 * Docs improvement: highlight DAG examples in README by @iancmoritz and @jlaneve in astronomer#695 (cherry picked from commit 2878d6a)

@jbandoro

[Justin Bandoro](https://www.linkedin.com/in/justin-bandoro-592b14a7/) (@jbandoro) is a Data Engineer at Kevala Inc. He's based in San Francisco (USA) and has been an early adopter of Cosmos, using it regularly at his company. Not only has he been using Cosmos since the early stages, but he has consistently improved Cosmos since January 2023: ![Screenshot 2023-12-04 at 16 28 29](https://github.com/astronomer/astronomer-cosmos/assets/272048/43197938-d1ab-431f-b101-b6026e5cd3ab) Some of his contributions include new features, code quality, documentation and overall improvements. Some examples: * Speed up integration tests in 67% astronomer#732 * Prevent override of dbt profile fields astronomer#702 * Add support for env vars in `RenderConfig` in astronomer#690 * Use symbolic links to run local tasks, avoiding to copy potentially huge dbt project folders in astronomer#660 * Improve documentation in astronomer#638 * Automated and improved the code complexity checks in astronomer#629 * Added `DbtDocsGCSOperator` in astronomer#616 * Added support for Python 3.7 in astronomer#88 and astronomer#214 Additionally, he has been interacting with users in the #airflow-dbt Slack channel in a very collaborative and supportive way. We want to promote him as a Cosmos committer and maintainer for all these, recognising his constant efforts and achievements towards our community. Thank you very much, @jbandoro !

prevent overriding dbt profile fields

34deee0

jbandoro requested a review from a team as a code owner November 23, 2023 01:51

jbandoro requested a review from a team November 23, 2023 01:51

jbandoro had a problem deploying to external November 23, 2023 01:51 — with GitHub Actions Error

dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. area:profile Related to ProfileConfig, like Athena, BigQuery, Clickhouse, Spark, Trino, etc labels Nov 23, 2023

Merge branch 'main' into 696-raise-exception-profile-args-overriding-…

ebb5e16

…reserved

tatiana had a problem deploying to external November 23, 2023 13:57 — with GitHub Actions Failure

tatiana approved these changes Nov 23, 2023

View reviewed changes

tatiana added this to the 1.2.5 milestone Nov 23, 2023

tatiana merged commit 8f7a04b into astronomer:main Nov 23, 2023

This was referenced Nov 23, 2023

DO NOT MERGE - Release 1.2.5 #708

Closed

Release 1.2.5 #709

Merged

Release 1.2.5 #710

Merged

tatiana mentioned this pull request Dec 4, 2023

Promote @jbandoro to committer #744

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent overriding dbt profile fields with profile args of "type" or "method"#702

Prevent overriding dbt profile fields with profile args of "type" or "method"#702
tatiana merged 2 commits into
astronomer:mainfrom
jbandoro:696-raise-exception-profile-args-overriding-reserved

jbandoro commented Nov 23, 2023

Uh oh!

netlify Bot commented Nov 23, 2023 •

edited

Loading

Uh oh!

tatiana left a comment

Uh oh!

tatiana commented Nov 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	def profile(self) -> dict[str, Any \| None]:
	"""
	Generates a GCP profile.
	Even though the Airflow connection contains hard-coded Service account credentials,
	we generate a temporary file and the DBT profile uses it.
	"""
	return {
	**self.mapped_params,
	"threads": 1,
	**self.profile_args,
	}

Conversation

jbandoro commented Nov 23, 2023

Description

Related Issue(s)

Breaking Change?

Checklist

Uh oh!

netlify Bot commented Nov 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Uh oh!

tatiana left a comment

Choose a reason for hiding this comment

Uh oh!

tatiana commented Nov 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

netlify Bot commented Nov 23, 2023 •

edited

Loading