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

Use targeted database for BigQuery profile #103

Merged

Conversation

jbandoro
Copy link
Collaborator

Fixes #102, the diffs in this PR are from the pre-commit black formatter, when it was actually only a single line update. I tested and it works as expected, though not sure if we want to default to the service-account project if db_name is None.

@jbandoro jbandoro requested a review from a team as a code owner January 26, 2023 20:58
@chrishronek
Copy link
Contributor

@jbandoro wanted to confirm this value would be the equivalent of a GCP project_id, but your change will make it configurable, correct?

@chrishronek chrishronek added the enhancement New feature or request label Jan 26, 2023
@jbandoro
Copy link
Collaborator Author

@jbandoro wanted to confirm this value would be the equivalent of a GCP project_id, but your change will make it configurable, correct?

yes that's correct. ATM the target GCP project_id is that of the GCP service account, and this PR will allow the project_id (dbt database) to be specified in dbt_args, for the case where a service account has access to many GCP projects.

@chrishronek
Copy link
Contributor

@jbandoro cool, I think it makes sense to defer a project_id (or database) from the service account when the db_name isn't specified, which is why I added that last commit. Thank you for this contribution; it makes a lot of sense and is much appreciated! I will have @jlaneve take a look before we approve, just so that my commit is also getting reviewed. Once we get his 👍 then we can merge!

Copy link
Collaborator

@jlaneve jlaneve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, this change looks good! eventually as the connection logic continues to grow we may want to find a better/cleaner way to define this, but I think this is good for now

@chrishronek chrishronek merged commit d7d88e6 into astronomer:main Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dbt_args with "db_name" target is not used for BigQuery profiles
3 participants