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

Default to current working directory for profiles.yml and fall back to ~/.dbt #5717

Merged
merged 20 commits into from Sep 20, 2022

Conversation

dbeatty10
Copy link
Contributor

@dbeatty10 dbeatty10 commented Aug 26, 2022

resolves #5411

uses #5715

Description

Use the following priority order to search for the profiles.yml configuration file:

  1. --profiles-dir command-line argument
  2. DBT_PROFILES_DIR environment variable
  3. current working directory 👈 NEW
  4. HOME directory of the user (i.e. ~/.dbt/)

Checklist

Outstanding questions

  • We're testing that dbt debug produces the correct output; how many other subcommands do we need to test?
  • Do we need to test that core/dbt/lib.py handles this new case properly? If so, then how?
  • Is there any special testing that needs to be done for dbt Cloud or dbt Server?

Some non-critical curiosities while we're here:

@cla-bot cla-bot bot added the cla:yes label Aug 26, 2022
@github-actions
Copy link
Contributor

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

@jtcohen6
Copy link
Contributor

@dbeatty10 Everything here makes sense to me! One point of clarification, and responses to your questions.

"Look here first" - which "here"?

  1. current working directory of dbt_project.yml NEW

As discussed during our meeting on Wednesday, this should just be the current working directory. In 95% of cases, we should expect the current working directory == the directory of the active root project (dbt_project.yml). In cases where users set --project-dir, I think the cleanest behavior is:

  • They also set a custom profile dir via CLI flag or env var → use it!
  • They don't set a custom profile dir → use the current working directory (rather than implicitly taking direction from --project-dir), then fall back to the HOME directory

Questions

We're testing that dbt debug produces the correct output; how many other subcommands do we need to test?

Seems sufficient to me! dbt debug uses the same standard RuntimeConfig as all other tasks (except deps + clean, which don't use a profile at all).

Do we need to test that core/dbt/lib.py handles this new case properly? If so, then how?

I don't think so. The profiles_dir is manually set there currently, and only allows two options: env var or home dir.

dbt-core/core/dbt/lib.py

Lines 15 to 18 in f5a94fc

if os.getenv("DBT_PROFILES_DIR"):
profiles_dir = os.getenv("DBT_PROFILES_DIR")
else:
profiles_dir = os.path.expanduser("~/.dbt")

Is there any special testing that needs to be done for dbt Cloud or dbt Server?

dbt Cloud uses the --profiles-dir CLI flag, which always takes top precedence. I just checked to confirm.


Help text

I realized our CLI help text for --profiles-dir includes the default location (HOME dir). Should we update it to be "If not set, dbt will look in the current working directory first, otherwise the HOME?"

We have an old + new CLI now :)

dbt-core/core/dbt/main.py

Lines 261 to 264 in f5a94fc

Which directory to look in for the profiles.yml file. Default = {}
""".format(
DEFAULT_PROFILES_DIR
),

help=f"Which directory to look in for the profiles.yml file. Default = {PurePath.joinpath(Path.home(), '.dbt')}",

@dbeatty10
Copy link
Contributor Author

Okay, I've made all those updates @jtcohen6:

  1. Just be the current working directory (regardless of the location of the project directory)
  2. Updated help text in both CLIs
    1. Updates to new CLI to align with current CLI (since the current CLI has no default values for --profiles-dir and --project-dir)
  3. Remove a couple unused variables

@jtcohen6
Copy link
Contributor

Ready to rock & roll, whether here or there

@dbeatty10 dbeatty10 changed the title Project profiles Default to current working directory for profiles.yml and fall back to ~/.dbt Sep 16, 2022
@dbeatty10 dbeatty10 added Team:Execution ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering labels Sep 16, 2022
@dbeatty10 dbeatty10 marked this pull request as ready for review September 16, 2022 19:31
@dbeatty10 dbeatty10 requested a review from a team September 16, 2022 19:31
@dbeatty10 dbeatty10 requested review from a team as code owners September 16, 2022 19:31
@lostmygithubaccount
Copy link
Contributor

this is gonna make quickstart guides so much easier!

@leahwicz
Copy link
Contributor

@dbeatty10 does that change anything to this doc #5425?

Sorry I owe you a review here...

@dbeatty10
Copy link
Contributor Author

@dbeatty10 does that change anything to this doc #5425?

@leahwicz I'll double check if that ADR needs any changes.

This PR does implement this exactly:
image

However, we did not implement anything in relation to the non-existent DBT_PROJECT_DIR environment variable described here:
image

Copy link
Contributor

@gshank gshank left a comment

Choose a reason for hiding this comment

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

If you could merge main to handle the merge conflicts message, this looks good to me!

@dbeatty10
Copy link
Contributor Author

@gshank merge conflicts handled now 👍

@gshank gshank merged commit bbf4fc3 into main Sep 20, 2022
@gshank gshank deleted the dbeatty/project-profiles branch September 20, 2022 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes 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-777] Default to current working directory for profiles.yml
7 participants