Skip to content

feat: use DBT_PROFILES_DIR env var as dbt profiles dir (if defined)#256

Merged
bastienboutonnet merged 5 commits intobitpicky:mainfrom
stumelius:dbt-profiles-dir-from-env-var
May 6, 2021
Merged

feat: use DBT_PROFILES_DIR env var as dbt profiles dir (if defined)#256
bastienboutonnet merged 5 commits intobitpicky:mainfrom
stumelius:dbt-profiles-dir-from-env-var

Conversation

@stumelius
Copy link
Contributor

@stumelius stumelius commented May 6, 2021

Description

Implemented support for the DBT_PROFILES_DIR env var. If the variable is not defined, the profiles dir defaults to ~/.dbt.

How was the change tested

I manually checked that the profiles directory was resolved as DBT_PROFILES_DIR env var. Unfortunately, I didn't write any unit tests as the change seems trivial.

Issue Information

Closes #255

Checklist

(Ideally, all boxes are checked by the time we merged the PR, if you don't know how to do any of these don't hesitate to say so in the PR and we'll help you out.)

  • I formatted my PR name according to CONTRIBUTING.md
  • I added a news fragment to help populating the changelog as encouraged in CONTRIBUTING.md
  • I added "Closes #<issue_number>" in the "Issue Information" section (if no issue, feel free to tick thick the box anyway).

@bastienboutonnet bastienboutonnet changed the title Use DBT_PROFILES_DIR env var as dbt profiles dir (if defined) feat: use DBT_PROFILES_DIR env var as dbt profiles dir (if defined) May 6, 2021
@bastienboutonnet
Copy link
Member

Thanks a lot for the PR @smomni

As you say I think it's ok not to have a unit test on this as it's trivial and that line is kind of covered. I might write a test case for it at a later point though.

It looks like the CI is failing for a weird reason so I'm gonna take a quick looksie.

In the meantime would you mind writing a news fragment for our changelog? you can follow the instructions here to do that: https://github.com/bitpicky/dbt-sugar/blob/main/CONTRIBUTING.md#shout-out-your-hardwork-in-the-changelog

It's going to be a feature so the name of that news fragment file should be 256.feature.md

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented May 6, 2021

Sourcery Code Quality Report

❌  Merging this PR will decrease code quality in the affected files by 1.30%.

Quality metrics Before After Change
Complexity 3.45 ⭐ 3.45 ⭐ 0.00
Method Length 50.40 ⭐ 51.30 ⭐ 0.90 👎
Working memory 7.84 🙂 8.31 🙂 0.47 👎
Quality 75.17% 73.87% 🙂 -1.30% 👎
Other metrics Before After Change
Lines 185 185 0
Changed files Quality Before Quality After Quality Change
dbt_sugar/core/clients/dbt.py 75.17% ⭐ 73.87% 🙂 -1.30% 👎

Here are some functions in these files that still need a tune-up:

File Function Complexity Length Working Memory Quality Recommendation
dbt_sugar/core/clients/dbt.py DbtProject.__init__.read_project.DbtProfile.read_profile 12 🙂 127 😞 12 😞 50.39% 🙂 Try splitting into smaller methods. Extract out complex expressions

Legend and Explanation

The emojis denote the absolute quality of the code:

  • ⭐ excellent
  • 🙂 good
  • 😞 poor
  • ⛔ very poor

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.


Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Let us know what you think of it by mentioning @sourcery-ai in a comment.

@stumelius
Copy link
Contributor Author

@bastienboutonnet There's now a changelog entry :)

@bastienboutonnet
Copy link
Member

bastienboutonnet commented May 6, 2021

Awesome @smomni ! Thanks a lot for this. Strangely enough the CI passes locally so that's a bit strange.

I'm gonna to ahead and merge your branch. I'm planning to make a new alpha release soon, but we (long story) merged what looks like a regression on master so we'll want to fix that before we release.

I'll ping you in the issue when that is done if you want?

Copy link
Member

@bastienboutonnet bastienboutonnet left a comment

Choose a reason for hiding this comment

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

Really nice work thanks @smomni

@bastienboutonnet bastienboutonnet merged commit b52472f into bitpicky:main May 6, 2021
@stumelius
Copy link
Contributor Author

Thanks, this was probably the smallest contribution I've ever made 😄 Sure thing, ping me if there's anything!

@bastienboutonnet
Copy link
Member

@smomni the new alpha build contains your changes: https://github.com/bitpicky/dbt-sugar/releases/tag/v0.1.0-a.3

Thanks a lot for the hard work and happy testing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dbt_sugar.core.exceptions.DbtProfileFileMissing: DBT_PROFILES_DIR env var is ignored

2 participants