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.

Sign up for GitHub

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

Jump to bottom

dbt run works with Click #6396

Merged
merged 21 commits into from
Dec 8, 2022
Merged

dbt run works with Click #6396

merged 21 commits into from
Dec 8, 2022

Conversation

iknox-fa
Copy link
Contributor

@iknox-fa iknox-fa commented Dec 6, 2022 •
edited
Loading

resolves #5551

Description

dbt run works with Click. Have fun folks!

Caveats:

  • Integration tests are failing. This is expected-- we'll get them sorted shortly in another PR.
  • The "old" cli is officially broken by this PR. Use the following command to run the new cli from the command line:
    python3 -m dbt.cli.main run --project-dir ../path/to/project
  • If you wish to compare the old and new methods look through the commits-- 703b7e5 specifically contains both old and new code paths for the entire PR.
  • This already has the changes in Profile works w Click #6336 merged in. If you wait until that's merged you'll have less to review here (if github updates the PR... I notice that doesn't always happen /shrug)

Checklist

iknox-fa and others added 14 commits December 4, 2022 14:31
@iknox-fa
support lower cased flags for legacy args compat
ba19357
@iknox-fa
added single-threaded param and removed late assignment of said arg
903f6c8
@iknox-fa
Combines Chenyu PR with mine
c2d2255
@MichelleArk @iknox-fa
Set Flags from UserConfig (#6266)
ae77041 
flags with user config, flags.WHICH from invoked_subcommand if available
@iknox-fa
fix unit tests
7bd9d63
@iknox-fa
Changelog
5f1131f
@iknox-fa
commit before merging with profile branch
5fc9c82
@iknox-fa
commit before merging with profile branch
0334911
@iknox-fa
Merge branch 'iknox/CT-912-GH-5536-Profile-for-click' into iknox/CT-9…
2905484 
…27-GH-5551-dbt-run-works-w-click
@iknox-fa
swapped models param with select param per jerco
51f7031
@iknox-fa
multi-path parse vars works
328765d
@iknox-fa
multi-path single_threaded args works
42ddd39
@iknox-fa
multi-path fix selector_name -> selector param name change
57057b5
@iknox-fa
multi-path selection spec works RUN COMMAND WORKS
703b7e5
@cla-bot cla-bot bot added the cla:yes label Dec 6, 2022
@iknox-fa iknox-fa changed the base branch from main to feature/click-cli December 6, 2022 19:07
@iknox-fa iknox-fa marked this pull request as ready for review December 6, 2022 21:08
@iknox-fa iknox-fa requested a review from a team December 6, 2022 21:08
@iknox-fa iknox-fa requested review from a team as code owners December 6, 2022 21:08
ChenyuLInx
ChenyuLInx reviewed Dec 7, 2022
View reviewed changes
core/dbt/cli/params.py
@@ -270,6 +262,15 @@
default="default",
)

select = click.option(
Copy link
Contributor

Choose a reason for hiding this comment

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

like the new name

core/dbt/config/profile.py Outdated
@@ -399,11 +399,13 @@ def from_raw_profiles(
)

@classmethod
def render_from_args(
def render(
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess a lot of the diff will disapear once profile PR is in

ChenyuLInx
ChenyuLInx approved these changes Dec 7, 2022
View reviewed changes
Copy link
Contributor

@ChenyuLInx ChenyuLInx left a comment

Choose a reason for hiding this comment

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

Ops was intended to start review but ended up left a lot of comment, this looks good to me!

stu-k
stu-k approved these changes Dec 7, 2022
View reviewed changes
Copy link
Contributor

@stu-k stu-k left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

@ChenyuLInx ChenyuLInx force-pushed the iknox/CT-927-GH-5551-dbt-run-works-w-click branch from 21e5888 to 3ee0790 Compare December 8, 2022 02:04
@iknox-fa iknox-fa merged commit 1809852 into feature/click-cli Dec 8, 2022
@iknox-fa iknox-fa deleted the iknox/CT-927-GH-5551-dbt-run-works-w-click branch December 8, 2022 15:47
@jtcohen6 jtcohen6 mentioned this pull request Jan 3, 2023
Closed
This was referenced Jan 31, 2023
Merged
Closed
This was referenced Feb 8, 2023
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stu-k stu-k stu-k approved these changes

@ChenyuLInx ChenyuLInx ChenyuLInx approved these changes

@emmyoop emmyoop Awaiting requested review from emmyoop emmyoop was automatically assigned from dbt-labs/core-team

@MichelleArk MichelleArk Awaiting requested review from MichelleArk

Assignees
No one assigned
Labels
cla:yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CT-927] dbt run works in click
4 participants
@iknox-fa @ChenyuLInx @stu-k @MichelleArk