Fix profile change not updating target #1665
Conversation
WalkthroughThe change updates the file system watcher handler in the Changes
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn config production Use Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 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)
🧰 Additional context used🧬 Code Graph Analysis (1)src/dbt_client/dbtCoreIntegration.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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.
Caution
Changes requested ❌
Reviewed everything up to 250b25b in 1 minute and 23 seconds. Click for details.
- Reviewed
20lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_MBOYfo5D0iV7hex8
You can customize 
| this.rebuildManifest(), | ||
| ), | ||
| // when the profile changes we need to refresh the project configuration | ||
| ...setupWatcherHandler(dbtProfileWatcher, async () => { |
There was a problem hiding this comment.
Consider adding error handling (e.g. a try-catch) inside the async watcher callback to log or handle failures from refreshProjectConfig or rebuildManifest. This avoids unhandled promise rejections if one of these operations fails.
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that changes to profiles.yml trigger a full project configuration refresh before rebuilding the manifest, keeping settings like targetName and modelPaths up to date.
- Update the watcher handler to call
refreshProjectConfig()thenrebuildManifest() - Introduce
refreshProjectConfig()inDBTCoreProjectIntegration - Maintain existing tests and linting
Comments suppressed due to low confidence (1)
src/dbt_client/dbtCoreIntegration.ts:496
- Add a unit or integration test that simulates a
profiles.ymlchange and verifies that bothrefreshProjectConfigandrebuildManifestare invoked as expected.
...setupWatcherHandler(dbtProfileWatcher, async () => {
| await this.refreshProjectConfig(); | ||
| await this.rebuildManifest(); |
There was a problem hiding this comment.
Consider adding a try/catch block inside the async watcher callback to handle potential errors from refreshProjectConfig or rebuildManifest, preventing unhandled promise rejections.
| await this.refreshProjectConfig(); | |
| await this.rebuildManifest(); | |
| try { | |
| await this.refreshProjectConfig(); | |
| await this.rebuildManifest(); | |
| } catch (error) { | |
| console.error("Error in watcher callback:", error); | |
| this.telemetry.sendTelemetryError("watcherCallbackError", error); | |
| } |
Copilot uses AI. Check for mistakes.
| ...setupWatcherHandler(dbtProfileWatcher, async () => { | ||
| await this.refreshProjectConfig(); | ||
| await this.rebuildManifest(); | ||
| }), |
There was a problem hiding this comment.
[nitpick] Extract the inline async callback into a named method (e.g., handleProfileChange) to improve readability and allow direct testing of the handler logic.
| ...setupWatcherHandler(dbtProfileWatcher, async () => { | |
| await this.refreshProjectConfig(); | |
| await this.rebuildManifest(); | |
| }), | |
| ...setupWatcherHandler(dbtProfileWatcher, this.handleProfileChange.bind(this)), |
Copilot uses AI. Check for mistakes.
|
As profiles can be located outside of the workspace folders, the vscode file watcher won't work. We should use a node js file watcher instead. |
Summary
fix for #1588
Testing
npm testnpm run lintImportant
Refresh project configuration in
DBTCoreProjectIntegrationwhenprofiles.ymlchanges to keep settings up-to-date.setupWatcherHandlerinDBTCoreProjectIntegrationto callrefreshProjectConfig()andrebuildManifest()whenprofiles.ymlchanges.refreshProjectConfig()toDBTCoreProjectIntegrationto update project configuration details liketargetName,targetPath,modelPaths, etc.npm testandnpm run lintto ensure functionality and code quality.This description was created by
for 250b25b. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit