vscode-dbt-power-user
vscode-dbt-power-user copied to clipboard
fix: resolve `DBT_PROFILES_DIR` correctly if relative path specified
Overview
Problem
if DBT_PROFILES_DIR is set, then
https://github.com/AltimateAI/vscode-dbt-power-user/blob/5735fa6e8f1ce3cbc2bb2b2d6f4a062baa2bf254/dbt_core_integration.py#L220
is joining project_dir with profiles_dir. If project_dir is coming from DBT_PROJECT_DIR, and both paths are relative then this is effectively producing <DBT_PROJECT_DIR>/<DBT_PROFILES_DIR> which may not exist.
In my case, I have
DBT_PROJECT_DIR=my_project
DBT_PROFILES_DIR=my_project/my_profiles
and this method is returning default_profiles_dir=my_project/my_project/my_profiles
The value of project_dir being passed is resolved as **/dbt_project.yml as in https://github.com/AltimateAI/vscode-dbt-power-user/blob/5735fa6e8f1ce3cbc2bb2b2d6f4a062baa2bf254/src/manifest/dbtWorkspaceFolder.ts#L90
see https://github.com/AltimateAI/vscode-dbt-power-user/issues/1554#issuecomment-2638276756 for the original reply
Closes #1554
Solution
Removing the unnecessary path join
Screenshot/Demo
A picture is worth a thousand words. Please highlight the changes if applicable.
How to test
- Steps to be followed to verify the solution or code changes
- Mention if there is any settings configuration added/changed/deleted
Checklist
- [ ] I have run this code and it appears to resolve the stated issue
- [ ]
README.mdupdated and added information about my change
[!IMPORTANT] Fixes path resolution in
default_profiles_dir()by removing unnecessary path joining withproject_dirwhenDBT_PROFILES_DIRis set.
- Behavior:
- Fixes path resolution in
default_profiles_dir()indbt_core_integration.pyby removing unnecessaryos.path.joinwithproject_dirwhenDBT_PROFILES_DIRis set.- Ensures
DBT_PROFILES_DIRis used as is if it's an absolute path, or resolved correctly if relative.- Misc:
- Closes issue #1554 related to incorrect profile directory resolution.
This description was created by
for d3864a8474d8fe172a6759c0e40e67bbb1f5d67e. It will automatically update as commits are pushed.
Summary by CodeRabbit
- Refactor
- Updated the resolution method for profile directories, allowing for direct use of custom configuration values.
- This change may impact scenarios involving relative paths in configuration settings.
Walkthrough
This pull request modifies the default_profiles_dir function in dbt_core_integration.py. Instead of joining the project directory with the DBT_PROFILES_DIR environment variable value, the function now returns the normalized path of the DBT_PROFILES_DIR value alone. No other code sections were altered.
Changes
| File(s) | Change Summary |
|---|---|
| dbt_core_integration.py | Updated the default_profiles_dir function: changed its return from joining project_dir with profiles_dir to returning the normalized path of profiles_dir alone. |
Assessment against linked issues
| Objective | Addressed | Explanation |
|---|---|---|
| Correctly resolve and use the DBT profiles directory from the environment (.env) file (#1554) | ✅ |
Possibly related PRs
- AltimateAI/vscode-dbt-power-user#1535: Modifies the same
default_profiles_dirfunction, with clarifications on handling relative paths which directly aligns with the changes in this PR.
[!TIP]
🌐 Web search-backed reviews and chat
- We have enabled web search-based reviews and chat for all users. This feature allows CodeRabbit to access the latest documentation and information on the web.
- You can disable this feature by setting
web_search: falsein theknowledge_basesettings.- Please share any feedback in the Discord discussion.
📜 Recent review details
Configuration used: CodeRabbit UI Review profile: CHILL Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between d3864a8474d8fe172a6759c0e40e67bbb1f5d67e and ee8e01096c753c740ac2759e88d51032b1f628ce.
📒 Files selected for processing (1)
dbt_core_integration.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- dbt_core_integration.py
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
🪧 Tips
Chat
There are 3 ways to chat with CodeRabbit:
- Review comments: Directly reply to a review comment made by CodeRabbit. Example:
I pushed a fix in commit <commit_id>, please review it.Generate unit testing code for this file.Open a follow-up GitHub issue for this discussion.
- Files and specific lines of code (under the "Files changed" tab): Tag
@coderabbitaiin a new review comment at the desired location with your query. Examples:@coderabbitai generate unit testing code for this file.@coderabbitai modularize this function.
- PR comments: Tag
@coderabbitaiin a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.@coderabbitai read src/utils.ts and generate unit testing code.@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.@coderabbitai help me debug CodeRabbit configuration file.
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.
CodeRabbit Commands (Invoked using PR comments)
@coderabbitai pauseto pause the reviews on a PR.@coderabbitai resumeto resume the paused reviews.@coderabbitai reviewto trigger an incremental review. This is useful when automatic reviews are disabled for the repository.@coderabbitai full reviewto do a full review from scratch and review all the files again.@coderabbitai summaryto regenerate the summary of the PR.@coderabbitai generate docstringsto generate docstrings for this PR. (Beta)@coderabbitai resolveresolve all the CodeRabbit review comments.@coderabbitai configurationto show the current CodeRabbit configuration for the repository.@coderabbitai helpto get help.
Other keywords and placeholders
- Add
@coderabbitai ignoreanywhere in the PR description to prevent this PR from being reviewed. - Add
@coderabbitai summaryto generate the high-level summary at a specific location in the PR description. - Add
@coderabbitaianywhere in the PR title to generate the title automatically.
CodeRabbit Configuration File (.coderabbit.yaml)
- You can programmatically configure CodeRabbit by adding a
.coderabbit.yamlfile to the root of your repository. - Please see the configuration documentation for more information.
- If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation:
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
Documentation and Community
- Visit our Documentation for detailed information on how to use CodeRabbit.
- Join our Discord Community to get help, request features, and share feedback.
- Follow us on X/Twitter for updates and announcements.
This change will break things for other people.For the extension the CWD is not obvously set, compared to CLI where there is clear concept of CWD. Becasue of that we went with choosing the project directory as the CWD.
@diegoquintanav I am not sure how this resolves the original bug #1554 . Can you help explain?
This change will break things for other people.For the extension the CWD is not obvously set, compared to CLI where there is clear concept of CWD. Becasue of that we went with choosing the project directory as the CWD.
Hey thanks for checking this PR!
This change affects only the scenario where DBT_PROFILES_DIR is set as an environment variable and is not an absolute path. I've pasted my original reply to the issue in the PR description if that helps.
In what cases would you say it breaks behaviour for other people?
In the code that is removed in this PR the path is relative to the project root. In your case it is to be defined, by the calling code (probably the first workspace root directory). All people that have setup a relative path from the project root would have an issue if we merge this.
In the code that is removed in this PR the path is relative to the project root. In your case it is to be defined, by the calling code (probably the first workspace root directory). All people that have setup a relative path from the project root would have an issue if we merge this.
thanks for the follow-up @mdesmet, I'm not sure if I understand the scenario you present.
My assumptions are that
- vscode is run from
/home/me/myproject - my dbt project is in
/home/me/myproject/mydbtprojectwith a profiles file in/home/me/myproject/mydbtproject/profiles/profiles.yml - environment variable
DBT_PROJECT_DIRis set to./mydbtproject - environment variable
DBT_PROFILES_DIRis set to./mydbtproject/profiles
In this scenario, what would you say is the value of project_dir in https://github.com/AltimateAI/vscode-dbt-power-user/blob/5735fa6e8f1ce3cbc2bb2b2d6f4a062baa2bf254/dbt_core_integration.py#L203C26-L203C37?
- vscode is run from
/home/me/myproject- my dbt project is in
/home/me/myproject/mydbtprojectwith a profiles file in/home/me/myproject/mydbtproject/profiles/profiles.yml
/home/me/myproject/mydbtproject will be project_dir
- environment variable
DBT_PROJECT_DIRis set to./mydbtproject
We don't use this environment variable as it is not compatible with multi dbt project projects, but we can rethink that one.
- environment variable
DBT_PROFILES_DIRis set to./mydbtproject/profilesIn this scenario, what would you say is the value of
project_dirin5735fa6/dbt_core_integration.py#L203C26-L203C37?
/home/me/myproject/mydbtproject/mydbtproject/profiles will be the profiles dir. So for it to work you should set DBT_PROFILES_DIR to ./profiles/
I'm open to rethink this but in that case we probably should set the project dir to what is in DBT_PROJECT_DIR. but then also the project detection should be adapted.
Thanks @mdesmet for your input. If I set DBT_PROFILES_DIR to ./profiles/ then that breaks the native dbt behavior. In other words, commands like
DBT_PROFILES_DIR=./profiles/ DBT_PROJECT_DIR=./mydbtproject/ dbt ls -m foo
Would break (there's no file at ./profiles/profiles.yml). If the extension is using dbt reserved environment variables, then it should replicate the dbt behavior.
We don't use this environment variable as it is not compatible with multi dbt project projects, but we can rethink that one.
I think this is key. What is a multi-dbt project? does dbt support one natively through environment variables?
a hacky solution can be done, if the extension decides to use native dbtenvironment variables and support multi project environments, by doing in https://github.com/AltimateAI/vscode-dbt-power-user/blob/dc24d1e269a70bf44349346bf657a86e1af5bf5d/dbt_core_integration.py#L242C29-L242C40
if "DBT_PROFILES_DIR" in os.environ:
# case when DBT_PROFILES_DIR=~/something
profiles_dir = os.path.expanduser(os.environ["DBT_PROFILES_DIR"])
if os.path.isabs(profiles_dir):
# if path is absolute, then ok, go ahead
return os.path.normpath(profiles_dir)
if "DBT_PROJECT_DIR" in os.environ: # new
# assume DBT_PROFILES_DIR is relative to the same directory that DBT_PROJECT_DIR was set to
# return the profiles_dir path as it is
return os.path.normpath(profiles_dir)) # new
return os.path.normpath(os.path.join(project_dir, profiles_dir))
a different approach perhaps would be that if DBT_PROJECT_DIR is set, assume we are working only with one project and in
https://github.com/AltimateAI/vscode-dbt-power-user/blob/5735fa6e8f1ce3cbc2bb2b2d6f4a062baa2bf254/src/manifest/dbtWorkspaceFolder.ts#L87-L90
resolve the argument passed to findFiles as DBT_PROJECT_DIR if present.
@mdesmet @saravmajestic Can you please review the code?
Thanks @mdesmet for your input. If I set
DBT_PROFILES_DIRto./profiles/then that breaks the nativedbtbehavior. In other words, commands likeDBT_PROFILES_DIR=./profiles/ DBT_PROJECT_DIR=./mydbtproject/ dbt ls -m fooWould break (there's no file at
./profiles/profiles.yml). If the extension is usingdbtreserved environment variables, then it should replicate thedbtbehavior.We don't use this environment variable as it is not compatible with multi dbt project projects, but we can rethink that one.
I think this is key. What is a multi-dbt project? does
dbtsupport one natively through environment variables?a hacky solution can be done, if the extension decides to use native
dbtenvironment variables and support multi project environments, by doing indc24d1e/dbt_core_integration.py#L242C29-L242C40if "DBT_PROFILES_DIR" in os.environ: # case when DBT_PROFILES_DIR=~/something profiles_dir = os.path.expanduser(os.environ["DBT_PROFILES_DIR"]) if os.path.isabs(profiles_dir): # if path is absolute, then ok, go ahead return os.path.normpath(profiles_dir) if "DBT_PROJECT_DIR" in os.environ: # new # assume DBT_PROFILES_DIR is relative to the same directory that DBT_PROJECT_DIR was set to # return the profiles_dir path as it is return os.path.normpath(profiles_dir)) # new return os.path.normpath(os.path.join(project_dir, profiles_dir))
This can work but I would add a check if the path exists and then only go in this path, this will act as an additional safeguard.
a different approach perhaps would be that if
DBT_PROJECT_DIRis set, assume we are working only with one project and inhttps://github.com/AltimateAI/vscode-dbt-power-user/blob/5735fa6e8f1ce3cbc2bb2b2d6f4a062baa2bf254/src/manifest/dbtWorkspaceFolder.ts#L87-L90
resolve the argument passed to
findFilesasDBT_PROJECT_DIRif present.
This won't resolve the issue, as still the profiles dir will be evaluated against the project dir.