fix: logs for lineage error #1517
Conversation
WalkthroughThe pull request introduces changes to two files: Changes
Suggested reviewers
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.
👍 Looks good to me! Reviewed everything up to eb9f997 in 21 seconds
More details
- Looked at
43lines of code in2files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. src/webview_provider/newLineagePanel.ts:479
- Draft comment:
Consider usingdebuginstead ofinfofor this log statement, as it is primarily for debugging purposes. - Reason this comment was not posted:
Confidence changes required:50%
The logging statements added in thegetStartingNodemethod are useful for debugging, but the log level used isinfo. It might be more appropriate to usedebugfor these logs, as they are primarily for development and debugging purposes, not for general information logging.
2. src/webview_provider/newLineagePanel.ts:490
- Draft comment:
Consider usingdebuginstead ofinfofor this log statement, as it is primarily for debugging purposes. - Reason this comment was not posted:
Confidence changes required:50%
The logging statements added in thegetStartingNodemethod are useful for debugging, but the log level used isinfo. It might be more appropriate to usedebugfor these logs, as they are primarily for development and debugging purposes, not for general information logging.
3. src/webview_provider/newLineagePanel.ts:476
- Draft comment:
Please specify a return type for the functiongetStartingNode. This will help in refactoring and understanding the code better. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_shAGTets1sgKjiXs
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
🧹 Outside diff range and nitpick comments (2)
src/manifest/dbtWorkspaceFolder.ts (1)
85-92: LGTM! Consider extracting the exclude pattern as a constant.The implementation correctly excludes
dbt_packagesandsite-packagesdirectories from project discovery, which will improve performance by avoiding unnecessary scans.Consider extracting the exclude pattern as a class constant for better maintainability:
+ private static readonly PROJECT_EXCLUDE_PATTERN = "**/{dbt_packages,site-packages}"; async discoverProjects() { - const excludePattern = "**/{dbt_packages,site-packages}"; + const excludePattern = DBTWorkspaceFolder.PROJECT_EXCLUDE_PATTERN;src/webview_provider/newLineagePanel.ts (1)
489-492: LGTM! Consider adding null check for tableName.The added logging will help track cases where nodes cannot be found. However,
tableNameis derived fromgetFilename()which assumeswindow.activeTextEditoris not null.Consider adding a null check:
const tableName = this.getFilename(); + if (!tableName) { + this.dbtTerminal.info("Lineage:getStartingNode", "No active file"); + return { + aiEnabled, + missingLineageMessage: this.getMissingLineageMessage(), + }; + } const _node = nodeMetaMap.lookupByBaseName(tableName);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/manifest/dbtWorkspaceFolder.ts(1 hunks)src/webview_provider/newLineagePanel.ts(2 hunks)
🔇 Additional comments (1)
src/webview_provider/newLineagePanel.ts (1)
479-479: LGTM! Appropriate logging for event not found scenario.
The added log message will help in diagnosing issues when lineage data is unavailable.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 8375d7a in 19 seconds
More details
- Looked at
16lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. src/webview_provider/newLineagePanel.ts:463
- Draft comment:
Consider providing a more descriptive error message to give better context in the logs. For example, include information about the operation being attempted or the state of the application. - Reason this comment was not posted:
Confidence changes required:50%
The error logging added in the catch block is a good addition for debugging purposes. However, the error message should be more descriptive to provide better context when reviewing logs.
2. src/webview_provider/newLineagePanel.ts:460
- Draft comment:
Please specify a return type for thegetMissingLineageMessagefunction to ensure easier refactoring and better readability. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_aiBbtvJ2xkO9xCLp
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/webview_provider/newLineagePanel.ts (1)
494-497: LGTM! Good logging implementation with context.The added logging with table name context will help track cases where nodes cannot be found, aiding in debugging lineage issues.
Consider refactoring the method to reduce duplication in the error handling paths:
private getStartingNode(): | { node?: Table; aiEnabled: boolean; missingLineageMessage?: { message: string; type: string }; } | undefined { const aiEnabled = this.altimate.enabled(); + const getErrorResponse = (message: string) => ({ + aiEnabled, + missingLineageMessage: this.getMissingLineageMessage(), + }); + const event = this.queryManifestService.getEventByCurrentProject(); if (!event?.event) { this.dbtTerminal.info("Lineage:getStartingNode", "No event found"); - return { - aiEnabled, - missingLineageMessage: this.getMissingLineageMessage(), - }; + return getErrorResponse("No event found"); } const { nodeMetaMap } = event.event; const tableName = this.getFilename(); const _node = nodeMetaMap.lookupByBaseName(tableName); if (!_node) { this.dbtTerminal.info( "Lineage:getStartingNode", `No node found for ${tableName}`, ); - return { - aiEnabled, - missingLineageMessage: this.getMissingLineageMessage(), - }; + return getErrorResponse(`No node found for ${tableName}`); } const key = _node.uniqueId; const url = window.activeTextEditor!.document.uri.path; const node = this.dbtLineageService.createTable(event.event, url, key); return { node, aiEnabled }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/webview_provider/newLineagePanel.ts(3 hunks)
🔇 Additional comments (2)
src/webview_provider/newLineagePanel.ts (2)
463-467: LGTM! Good error logging implementation.
The added error logging will help with debugging lineage issues, particularly when diagnostics errors occur. The implementation properly handles error types and logs both the message and full error object.
484-484: LGTM! Appropriate logging for event availability.
The info-level logging is well-placed to track when lineage cannot be generated due to missing events. This will help diagnose user-reported issues where lineage fails to display.
Overview
Problem
Solution
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
Optimize project discovery and add logging for error tracking in
dbtWorkspaceFolder.tsandnewLineagePanel.ts.discoverProjects()indbtWorkspaceFolder.tsby excludingdbt_packagesandsite-packagesdirectories.getStartingNode()innewLineagePanel.tsto log when no event or node is found.discoverProjects()indbtWorkspaceFolder.ts.This description was created by
for 8375d7a. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
dbt_packagesandsite-packages) from the search.Bug Fixes