feat: allow to specify dbt projects directly and skip discovery #1593
Conversation
this allows to skip the project discovery as it often fails to discover projects --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/OneCyrus/vscode-dbt-power-user?shareId=XXXX-XXXX-XXXX-XXXX).
WalkthroughThis pull request introduces a new configuration property Changes
Sequence Diagram(s)sequenceDiagram
participant DP as DiscoverProjects()
participant WC as Workspace Configuration
participant URI as URI Constructor
participant OL as Old Discovery Logic
DP->>WC: Retrieve "dbt.specifiedProjects" setting
alt Specified projects provided
WC-->>DP: Return non-empty array
DP->>URI: Convert each path string to URI
URI-->>DP: Return array of URIs
DP-->>Caller: Return specified project URIs
else No specified projects
WC-->>DP: Return empty array
DP->>OL: Execute default project discovery
OL-->>DP: Return discovered project URIs
DP-->>Caller: Return discovered project URIs
end
Possibly related PRs
Suggested reviewers
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 8133da5 in 1 minute and 31 seconds
More details
- Looked at
57lines of code in3files - Skipped
0files when reviewing. - Skipped posting
11drafted comments based on config settings.
1. src/dbt_client/dbtCloudIntegration.ts:151
- Draft comment:
Great use of an early return to bypass discovery when 'dbt.specifiedProjects' is set. Consider updating documentation (README) to detail this new setting. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
2. src/dbt_client/dbtCoreIntegration.ts:178
- Draft comment:
Nice early return in 'discoverProjects' when 'dbt.specifiedProjects' is provided. Ensure that users are aware of absolute vs relative paths. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
3. package.json:287
- Draft comment:
New configuration property 'dbt.specifiedProjects' is added. Please update the README/documentation to explain how this setting works. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
4. src/dbt_client/dbtCloudIntegration.ts:151
- Draft comment:
Good use of an early return based on 'specifiedProjects'. Consider adding a debug log here to indicate that manual project specification is active, which would aid in troubleshooting. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
5. src/dbt_client/dbtCoreIntegration.ts:177
- Draft comment:
The early return based on 'specifiedProjects' is duplicated here. Consider refactoring this check into a shared utility function to reduce code duplication and improve maintainability. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
6. src/dbt_client/dbtCloudIntegration.ts:336
- Draft comment:
Typo found: 'Could not initalize Python environemnt' should be 'Could not initialize Python environment'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. src/dbt_client/dbtCloudIntegration.ts:1151
- Draft comment:
Typo found: 'Error occured while reading file:' should be 'Error occurred while reading file:'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. src/dbt_client/dbtCloudIntegration.ts:1233
- Draft comment:
Typo found: 'An error occured while parsing following json:' should be 'An error occurred while parsing the following JSON:'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. src/dbt_client/dbtCloudIntegration.ts:245
- Draft comment:
Typo found in comment: 'First time let,s block' should be 'First time let's block'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
10. src/dbt_client/dbtCoreIntegration.ts:64
- Draft comment:
Typographical error: In the comment on line 64, 'shouold' should be corrected to 'should'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. src/dbt_client/dbtCoreIntegration.ts:215
- Draft comment:
Typographical error: The word 'occured' appears in several error messages (e.g., near line 215, 520, and 1202). It should be spelled 'occurred'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_k4XDTbNBUAjpAXeq
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/dbt_client/dbtCoreIntegration.ts (1)
186-240: Consider adding logging for when using specified projects.The implementation is functionally correct, but adding debug logging (similar to line 214) specifically for when using specified projects would help with troubleshooting.
Consider adding logging when the specified projects are being used:
if (specifiedProjects.length > 0) { + this.dbtTerminal.debug( + "dbtCoreIntegration:discoverProjects", + `Using specified projects: ${specifiedProjects.join(', ')}`, + ); return specifiedProjects.map((projectPath) => Uri.file(projectPath)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(1 hunks)src/dbt_client/dbtCloudIntegration.ts(1 hunks)src/dbt_client/dbtCoreIntegration.ts(1 hunks)
🔇 Additional comments (3)
package.json (1)
287-294: Well-implemented configuration property for user-specified projects.This addition allows users to manually specify dbt project paths, which directly addresses the PR objective of bypassing the automatic project discovery feature when it fails. The property is correctly defined as an array of strings with an appropriate default value and clear description.
src/dbt_client/dbtCloudIntegration.ts (1)
153-159: Good implementation of the user-specified projects feature.This code correctly retrieves the specified projects from the workspace configuration and returns them as Uri objects if present, bypassing the automatic discovery logic. This is exactly what's needed to address the issue mentioned in the PR description.
The early return pattern used here is appropriate - if the user has explicitly specified projects, we return those immediately rather than attempting automatic discovery.
src/dbt_client/dbtCoreIntegration.ts (1)
178-184: Consistent implementation across DBT integrations.This implementation mirrors the approach used in the cloud integration, ensuring consistent behavior regardless of whether users are using dbt core or dbt cloud. The code correctly retrieves user-specified projects from the configuration and prioritizes them over automatic discovery.
|
@OneCyrus The auto discovery logic is designed to automatically identify projects. I was curious if you investigated why some projects aren't being discovered. I believe this could help us address the root cause of the problem without needing to pass any direct paths. |
|
Thanks @OneCyrus, have you tried to run the extension in debug mode and check why the projects are not detected in the first place? |
|
the issue is that the workspace.findFiles isn't somehow reliable. about 1 in 5 times it finds the files and the other 4 times it returns nothing. also our workspace is quite large with multiple dbt projects. maybe it just runs into a timeout randomly. unfortunately there is no error in the debug logs. |
|
@OneCyrus : Can you make the changes to retry the project detection? Also can you link the underlying issue to the vscode issue that you found where findInFiles is not reliable. |
Overview
this allows to skip the project discovery as it often fails to discover projects
Problem
the extension often didn't discover our projects and returned an empty list.
fixes #1437
Solution
allow to specify the dbt projects manually through a setting
dbt.specifiedProjectsHow to test
dbt.specifiedProjectsin settings.jsonChecklist
README.mdupdated and added information about my changeSummary by CodeRabbit