Get project name from dbt #1469
Conversation
WalkthroughThe changes in this pull request enhance project name management within the DBT integration classes. Key modifications include the addition of a private Changes
Possibly related PRs
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.
❌ Changes requested. Reviewed everything up to 83d5eac in 57 seconds
More details
- Looked at
119lines of code in4files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. src/dbt_client/dbtCloudIntegration.ts:177
- Draft comment:
Consider checking the Node.js version compatibility forcrypto.randomUUID()to ensure it works in the target environment. If compatibility is a concern, consider using an alternative method for generating UUIDs. - Reason this comment was not posted:
Comment did not seem useful.
2. src/dbt_client/dbtCoreIntegration.ts:246
- Draft comment:
Consider checking the Node.js version compatibility forcrypto.randomUUID()to ensure it works in the target environment. If compatibility is a concern, consider using an alternative method for generating UUIDs. - Reason this comment was not posted:
Marked as duplicate.
3. src/dbt_client/dbtCoreIntegration.ts:562
- Draft comment:
Please specify a return type for thefindAdapterTypefunction to improve code readability and maintainability. This also applies to other functions likefindVersion,findProjectName,findModelPaths,findSeedPaths,findMacroPaths,findTargetPath, andfindPackagesInstallPathin the same file. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_iR93AlFP1dRS3gE3
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.
| @@ -988,6 +993,15 @@ export class DBTCloudProjectIntegration | |||
| stderr, | |||
There was a problem hiding this comment.
Please specify a return type for the lookupValue function to improve code readability and maintainability. This also applies to the lookupEntries function in the same file.
| @@ -207,7 +207,7 @@ export class DBTProject implements Disposable { | |||
| } | |||
|
|
|||
| public getProjectName() { | |||
There was a problem hiding this comment.
Please specify a return type for the getProjectName function to improve code readability and maintainability.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
src/manifest/dbtProject.ts (3)
Line range hint
1-1000: Consider refactoring this large file for better maintainability.The
DBTProjectclass is quite large and contains many methods. Consider splitting it into smaller, more focused classes or modules to improve maintainability and readability. This could involve creating separate classes for different aspects of the project (e.g., compilation, execution, schema management).
Line range hint
1-1000: Address TODO comments throughout the file.There are several TODO comments in the file. It's recommended to address these or convert them into tracked issues in your project management system. This ensures that these tasks are not forgotten and can be prioritized appropriately.
Line range hint
1-1000: Minimize usage ofanytype for better type safety.There are instances of
anytype being used in the file (e.g., line 31:private projectConfig: any;). Consider replacing these with more specific types or interfaces to improve type safety and code clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- src/dbt_client/dbtCloudIntegration.ts (4 hunks)
- src/dbt_client/dbtCoreIntegration.ts (4 hunks)
- src/dbt_client/dbtIntegration.ts (1 hunks)
- src/manifest/dbtProject.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (9)
src/dbt_client/dbtCoreIntegration.ts (5)
246-246: LGTM: Good addition of theprojectNameproperty.The new private
projectNameproperty is well-implemented. The default value using a UUID ensures uniqueness even if the project name is not set immediately. This approach provides a good fallback and maintains consistency.
565-567: LGTM: Well-implementedgetProjectNamemethod.The new
getProjectNamemethod is a good addition. It provides controlled access to the privateprojectNameproperty, maintaining proper encapsulation. The method is simple and follows the expected behavior for a getter.
1069-1073: LGTM: Well-implementedfindProjectNamemethod.The new private async
findProjectNamemethod is a good addition. It correctly uses the Python bridge to retrieve the project name from the DBT configuration. The method is properly encapsulated and follows the async pattern, which is appropriate for potentially time-consuming operations.
382-382: LGTM: Good update torefreshProjectConfig.The
refreshProjectConfigmethod has been correctly updated to include a call tofindProjectName. This ensures that theprojectNameis properly initialized when the project configuration is refreshed, maintaining consistency with other project properties.
246-246: Overall, excellent additions to enhance project name functionality.The changes to add project name functionality to the
DBTCoreProjectIntegrationclass are well-implemented. The newprojectNameproperty,getProjectNamemethod, andfindProjectNamemethod are all properly designed and integrated into the existing code. The update torefreshProjectConfigensures consistency in initializing the project name along with other properties. These additions enhance the class's functionality without disrupting existing behavior and follow good coding practices.Also applies to: 565-567, 1069-1073, 382-382
src/manifest/dbtProject.ts (1)
210-210: LGTM! Improved project name retrieval.The change to
getProjectName()method looks good. It now delegates the responsibility of retrieving the project name to thedbtProjectIntegrationobject. This approach is more flexible and allows for different implementations of project name retrieval based on the integration type (e.g., DBT Core or DBT Cloud).src/dbt_client/dbtCloudIntegration.ts (3)
380-382: Implementation ofgetProjectNameis correctThe
getProjectNamemethod correctly returns theprojectName.
1024-1024: Assignment ofprojectNameusinglookupValueis appropriateThe assignment of
this.projectNameusinglookupValue("Project name")is correctly implemented.
996-1004: Verify the regular expression inlookupValuefunctionEnsure that the regular expression in the
lookupValuefunction accurately captures the desired value fromstdout. Consider edge cases wherestdoutmight not contain the expected format.Run the following script to test the regular expression against sample outputs:
✅ Verification successful
Regular expression in
lookupValuefunction works as expected
- Successfully captures the project name with standard and irregular spacing.
- Appropriately handles cases with multiple entries or missing values by not capturing, allowing error handling to proceed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Test the regex pattern used in 'lookupValue' function with sample outputs. # Expected: Should print the captured project name if the pattern matches. # Sample stdout content STDOUT_CONTENT=$(cat <<EOF Project name MyProject Model paths [models] Seed paths [seeds] EOF ) # Regex pattern echo "$STDOUT_CONTENT" | gawk 'match($0, /Project name\s*(.*)/, m) { print m[1] }'Length of output: 272
Script:
#!/bin/bash # Description: Test the regex pattern used in 'lookupValue' function with various edge case outputs. # Test Case 1: stdout does not contain "Project name" STDOUT_CONTENT=$(cat <<EOF Model paths [models] Seed paths [seeds] EOF ) echo "Test Case 1:" echo "$STDOUT_CONTENT" | gawk 'match($0, /Project name\s*(.*)/, m) { print m[1] }' # Test Case 2: "Project name" with irregular spacing STDOUT_CONTENT=$(cat <<EOF Project name : MyProject Model paths [models] Seed paths [seeds] EOF ) echo "Test Case 2:" echo "$STDOUT_CONTENT" | gawk 'match($0, /Project name\s*[:\-]?\s*(.*)/, m) { print m[1] }' # Test Case 3: Multiple "Project name" entries STDOUT_CONTENT=$(cat <<EOF Project name MyProject Project name AnotherProject Model paths [models] Seed paths [seeds] EOF ) echo "Test Case 3:" echo "$STDOUT_CONTENT" | gawk 'match($0, /Project name\s*(.*)/, m) { print m[1] }' # Test Case 4: "Project name" without an accompanying value STDOUT_CONTENT=$(cat <<EOF Project name Model paths [models] Seed paths [seeds] EOF ) echo "Test Case 4:" echo "$STDOUT_CONTENT" | gawk 'match($0, /Project name\s*(.*)/, m) { print m[1] }'Length of output: 1218
| @@ -336,6 +336,7 @@ export interface DBTProjectIntegration extends Disposable { | |||
| getPackageInstallPath(): string | undefined; | |||
| getAdapterType(): string | undefined; | |||
| getVersion(): number[] | undefined; | |||
| getProjectName(): string; | |||
There was a problem hiding this comment.
💡 Codebase verification
Missing getProjectName() Implementation in Implementing Classes
Several classes implementing DBTProjectIntegration do not include the newly added getProjectName() method. Please ensure that all implementing classes define this method to maintain the interface contract.
src/dbt_client/dbtCloudIntegration.tssrc/dbt_client/dbtCoreIntegration.ts
🔗 Analysis chain
LGTM! Verify implementations of DBTProjectIntegration.
The addition of the getProjectName(): string method to the DBTProjectIntegration interface is a good enhancement for standardizing project name retrieval across DBT integrations.
Ensure that all classes implementing DBTProjectIntegration are updated to include this new method. Run the following script to check for implementations:
| @@ -174,6 +174,7 @@ export class DBTCloudProjectIntegration | |||
| private static QUEUE_ALL = "all"; | |||
| private targetPath?: string; | |||
| private version: number[] | undefined; | |||
| private projectName: string = "unknown_" + crypto.randomUUID(); | |||
There was a problem hiding this comment.
Import the 'crypto' module
The crypto module is used but not imported. You need to import it to use crypto.randomUUID().
Apply this diff to fix the missing import:
+import * as crypto from 'crypto';Committable suggestion was skipped due to low confidence.
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
Add functionality to retrieve project name from dbt configurations in
DBTCloudProjectIntegrationandDBTCoreProjectIntegration.getProjectName()method toDBTCloudProjectIntegrationandDBTCoreProjectIntegrationto retrieve project name from dbt configurations.projectNameusing dbt configuration ininitializePaths()indbtCloudIntegration.tsandrefreshProjectConfig()indbtCoreIntegration.ts.DBTProjectIntegrationinterface indbtIntegration.tsto includegetProjectName().getProjectName()inDBTProjectindbtProject.tsto usedbtProjectIntegration.getProjectName().This description was created by
for 83d5eac. It will automatically update as commits are pushed.
Summary by CodeRabbit