Perform dbt deps on project load
#1559
Conversation
WalkthroughThis pull request introduces modifications to the dbt (data build tool) integration classes, focusing on dependency management and project configuration. The changes primarily affect the Changes
Sequence DiagramsequenceDiagram
participant DBTProject
participant DBTProjectIntegration
participant Telemetry
DBTProject->>DBTProject: Check depsInitialized
alt Dependencies not initialized
DBTProject->>Telemetry: Send telemetry event
DBTProject->>DBTProjectIntegration: installDeps()
DBTProjectIntegration-->>DBTProject: Execute dependency command
DBTProject->>DBTProject: Set depsInitialized = true
end
DBTProject->>DBTProjectIntegration: rebuildManifest()
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
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? 🪧 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.
❌ Changes requested. Reviewed everything up to 0788ccc in 1 minute and 8 seconds
More details
- Looked at
85lines of code in2files - Skipped
0files when reviewing. - Skipped posting
7drafted comments based on config settings.
1. src/dbt_client/dbtCloudIntegration.ts:233
- Draft comment:
Corrected a typo in the variable name frompathsInitalizedtopathsInitialized. This change improves code readability and consistency. - Reason this comment was not posted:
Confidence changes required:10%
The typo in the variable name 'pathsInitalized' was corrected to 'pathsInitialized'. This change is consistent with the rest of the code and improves readability.
2. src/dbt_client/dbtCloudIntegration.ts:395
- Draft comment:
Removed the unusedretryCountparameter fromrebuildManifestfunction, simplifying the function signature. - Reason this comment was not posted:
Confidence changes required:10%
TherebuildManifestfunction had an unusedretryCountparameter which was removed. This is a good change as it simplifies the function signature.
3. src/dbt_client/dbtCloudIntegration.ts:564
- Draft comment:
Updated thedepsfunction to execute the command and handle errors properly, ensuring correct functionality. - Reason this comment was not posted:
Confidence changes required:10%
Thedepsfunction was updated to execute the command and handle errors properly. This change aligns with the intended functionality of executing dbt commands and handling their output.
4. src/manifest/dbtProject.ts:658
- Draft comment:
ChangedinstallDepsto be asynchronous, which is appropriate for handling I/O operations efficiently. - Reason this comment was not posted:
Confidence changes required:10%
TheinstallDepsfunction was changed to be asynchronous, which is appropriate given that it likely involves I/O operations that can benefit from async handling.
5. src/dbt_client/dbtCloudIntegration.ts:395
- Draft comment:
Please specify a return type for therebuildManifestfunction to improve code readability and maintainability. - Reason this comment was not posted:
Comment was on unchanged code.
6. src/dbt_client/dbtCloudIntegration.ts:233
- Draft comment:
Use an early return to simplify the logic inrefreshProjectConfigby returning immediately ifpathsInitializedis true. - Reason this comment was not posted:
Comment was on unchanged code.
7. src/dbt_client/dbtCloudIntegration.ts:401
- Draft comment:
Use an early return after cancellingrebuildManifestCancellationTokenSourceto reduce nesting inrebuildManifest. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_13CTl3gRAekkPKWw
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @@ -645,7 +655,7 @@ export class DBTProject implements Disposable { | |||
| ); | |||
| } | |||
|
|
|||
| installDeps() { | |||
| async installDeps() { | |||
There was a problem hiding this comment.
Please specify a return type for the installDeps function to improve code readability and maintainability. This comment also applies to the installDbtPackages function on line 646.
![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/manifest/dbtProject.ts (1)
526-534: Consider enhancing error handling and logging.While the dependency installation logic is sound, the error handling could be improved:
- The warning message could be more descriptive
- The error should be logged through the terminal service for consistency
- Consider notifying the user about dependency installation failures
Apply this diff to improve error handling:
if (!this.depsInitialized) { try { await this.installDeps(); } catch (error: any) { - // this is best effort - console.warn("An error occured while installing dependencies", error); + this.terminal.warn( + "DbtProjectDepsInstallation", + "Failed to install dependencies. Some features may not work correctly.", + true, + error + ); + window.showWarningMessage( + "Failed to install dbt dependencies. Some features may not work correctly." + ); } this.depsInitialized = true; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/dbt_client/dbtCloudIntegration.ts(4 hunks)src/manifest/dbtProject.ts(3 hunks)
🔇 Additional comments (5)
src/dbt_client/dbtCloudIntegration.ts (3)
195-195: LGTM! Fixed typo in variable name.The spelling correction from
pathsInitalizedtopathsInitializedimproves code readability while maintaining the same functionality.Also applies to: 236-240
398-398: LGTM! Simplified method signature.The removal of the unused
retryCountparameter simplifies the interface while maintaining the core functionality.
567-571: LGTM! Implemented deps command support.The method now properly executes the deps command instead of throwing an unsupported error. This change enables dependency management in dbt cloud, which is essential for the PR's objective of performing
db depson project load.src/manifest/dbtProject.ts (2)
111-111: LGTM! Added state tracking for dependency initialization.The
depsInitializedflag helps prevent redundant dependency installations by tracking the initialization state.
Line range hint
658-663: LGTM! Clean implementation of dependency installation.The
installDepsmethod is well-implemented with:
- Proper telemetry tracking
- Clear command creation and execution
- Async/await pattern for handling promises
mdesmet
changed the title
db deps on project loaddbt deps on project load
Overview
Problem
Describe the problem you are solving. Mention the ticket/issue if applicable.
Solution
Describe the implemented solution. Add external references if needed.
Screenshot/Demo
A picture is worth a thousand words. Please highlight the changes if applicable.
How to test
Checklist
README.mdupdated and added information about my changeImportant
Ensure dependencies are installed on project load by updating
DBTProjectandDBTCloudProjectIntegrationclasses.DBTProjectnow installs dependencies on project load by callinginstallDeps()inrebuildManifest().DBTCloudProjectIntegration'sdeps()method now executesdbt depscommand and throws an error ifstderris present.rebuildManifest()inDBTCloudProjectIntegrationno longer acceptsretryCountparameter.installDeps()inDBTProjectis now asynchronous.pathsInitalizedtopathsInitializedinDBTCloudProjectIntegration.This description was created by
for 0788ccc. It will automatically update as commits are pushed.
Summary by CodeRabbit
Bug Fixes
New Features
Refactor