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

Allow missing profiles.yml for dbt deps and dbt init #7546

Merged
merged 6 commits into from
May 11, 2023

Conversation

dbeatty10
Copy link
Contributor

@dbeatty10 dbeatty10 commented May 8, 2023

resolves #7511

Description

Options considered

We considered four different approaches:

  1. Leave all behavior as-is for 1.5 and add a note to the migration guide so folks know how to make updates
  2. Use type=click.Path(exists=False) to ignore path validation
  3. Give dbt deps it's own custom version of the --profiles-dir parameter that ignores path validation (similar to Option 2 above)
  4. Remove @p.profiles_dir from dbt deps

Initial decision

The initial decision by @jtcohen6 was:

  • Option 2: switch the parameter to type=click.Path(exists=False) to turn off this validation for all commands.

New decision

But during implementation, I discovered an alternative click option named profiles_dir_exists_false that has that behavior (which was added for dbt debug), so I chose to re-use that in order to preserve validation for the rest of the subcommands.

During code review, it would be easy to go back to the initial decision if we want to.

Checklist

  • I have read the contributing guide and understand what's expected of me
  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • Docs changes are not required/relevant for this PR
  • I have run changie new to create a changelog entry

@dbeatty10 dbeatty10 requested review from a team as code owners May 8, 2023 13:38
@dbeatty10 dbeatty10 requested review from aranke and davidbloss and removed request for a team May 8, 2023 13:38
@cla-bot cla-bot bot added the cla:yes label May 8, 2023
@dbeatty10
Copy link
Contributor Author

@jtcohen6 I think we'll want the 1.5 backport label added to this.

@jtcohen6
Copy link
Contributor

jtcohen6 commented May 8, 2023

@dbeatty10 Agreed!

To provide a little window into my methods: The label needs to be added after the PR is merged in order to trigger the GH workflow, but I like to add it before the PR is merged, as a reminder & a form of bookkeeping :)

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!

@dbeatty10
Copy link
Contributor Author

Verified that the new CI checks added in 40ef3d8 will fail as reported in the regression case:

image

@dbeatty10
Copy link
Contributor Author

dbt init was not playing nice when I was trying to test it, and I opened #7594 as as a result.

In the meantime, I peeled out its test in 6f94a86 in favor of adding it back in when resolving #7594 and manually tested dbt init --profiles-dir does_not_exist ✅.

@dbeatty10 dbeatty10 merged commit 630cd3a into main May 11, 2023
@dbeatty10 dbeatty10 deleted the dbeatty/optional-profiles-dir branch May 11, 2023 01:24
@dbeatty10
Copy link
Contributor Author

@jtcohen6 do you know if this was backported or not?

@abhinav-kumar-17
Copy link

Any dates on when this fix would be released in 1.5?

emmyoop pushed a commit that referenced this pull request May 22, 2023
* Allow missing `profiles.yml` for `dbt deps` and `dbt init`

* Some commands allow the `--profiles-dir` to not exist

* Remove fix to verify that CI tests work

* Allow missing `profiles.yml` for `dbt deps` and `dbt init`

* CI is not finding any installed adapters

* Remove functional test for `dbt init`
emmyoop pushed a commit that referenced this pull request May 22, 2023
* Allow missing `profiles.yml` for `dbt deps` and `dbt init`

* Some commands allow the `--profiles-dir` to not exist

* Remove fix to verify that CI tests work

* Allow missing `profiles.yml` for `dbt deps` and `dbt init`

* CI is not finding any installed adapters

* Remove functional test for `dbt init`
emmyoop added a commit that referenced this pull request May 22, 2023
…7677)

* Allow missing `profiles.yml` for `dbt deps` and `dbt init`

* Some commands allow the `--profiles-dir` to not exist

* Remove fix to verify that CI tests work

* Allow missing `profiles.yml` for `dbt deps` and `dbt init`

* CI is not finding any installed adapters

* Remove functional test for `dbt init`

Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com>
@dbeatty10
Copy link
Contributor Author

@abhinav-kumar-17 we don't have a precise date yet, but it will be included in the 1.5.1 dbt-core release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants