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

feat: move --profile and --target to global config #7920

Closed
wants to merge 1 commit into from
Closed

feat: move --profile and --target to global config #7920

wants to merge 1 commit into from

Conversation

rexledesma
Copy link

@rexledesma rexledesma commented Jun 22, 2023

Resolves #7798.

Description

Move --target and --profile command line flags to global config. Also, allow these flags to be set by environment variable (DBT_TARGET, and DBT_PROFILE).

Checklist

@rexledesma rexledesma requested a review from a team as a code owner June 22, 2023 13:55
@rexledesma rexledesma requested a review from aranke June 22, 2023 13:55
@cla-bot
Copy link

cla-bot bot commented Jun 22, 2023

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @rexledesma

Copy link
Member

@aranke aranke left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for contributing!

There's an opportunity to simplify the code, but I'll leave whether to do it up to you.

tests/unit/test_cli_flags.py Outdated Show resolved Hide resolved
@cla-bot
Copy link

cla-bot bot commented Jun 22, 2023

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @rexledesma

@cla-bot
Copy link

cla-bot bot commented Jun 22, 2023

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @rexledesma

@dbeatty10
Copy link
Contributor

@rexledesma thanks for submitting this so quickly! 🤩

Could you fill out this super short and easy form? It's a one-time requirement for all our contributors.

Once you do that, then we can make make the CLA Bot happy 😎

@jtcohen6 jtcohen6 added cli ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering labels Jun 25, 2023
@dbeatty10
Copy link
Contributor

@cla-bot check

@cla-bot
Copy link

cla-bot bot commented Oct 27, 2023

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @rexledesma

@cla-bot
Copy link

cla-bot bot commented Oct 27, 2023

The cla-bot has been summoned, and re-checked this pull request!

@rexledesma rexledesma closed this Nov 1, 2023
@barton996
Copy link
Contributor

@rexledesma Are you happy for me to reopen this PR at some point and use your code as a starting point/solution? I have a signed CLA

@dbeatty10
Copy link
Contributor

@barton996 I'll create a quick starting point for you and share with you here in a few minutes.

@dbeatty10
Copy link
Contributor

Thanks for picking this up @barton996!

You can use these commits from my dbeatty/7798-global-configs branch as a starting point.

It shows how to do the first two bullet points here.

You'd then need to do this part (which is the most time-consuming):

Adding a unit test here to prove these flags are supported in the "global" position, in addition to the "subcommand" position. (We already have a test to verify that dbt should raise an error if the same flag is passed in both positions.)

Finally, you'd need to run changie new to generate a new changelog entry with your username.

@barton996
Copy link
Contributor

@dbeatty10 Cheers I'll take a look when I get a chance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-2649] [Feature] Move --target and --profile command line flags to global config
5 participants