Fix target names lookup #1477
Fix target names lookup #1477
Conversation
WalkthroughThe changes in this pull request focus on the 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.
❌ Changes requested. Reviewed everything up to 5dd6acc in 32 seconds
More details
- Looked at
17lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. dbt_core_integration.py:869
- Draft comment:
Please provide a return type for theget_target_namesfunction to improve clarity and ease of refactoring. - Reason this comment was not posted:
Comment was on unchanged code.
2. dbt_core_integration.py:872
- Draft comment:
Use semantic versioning (semver) for version comparisons instead of direct array index comparisons. This issue is also present in other parts of the code, such as lines 40, 47, 62, 76, 278, 386, 398, 438, 567, 576, 726, and 856. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_UGCIHLrEnuYyTPPG
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.
| project = profile[self.project_name] | ||
| if "outputs" in project: | ||
| outputs = project["outputs"] | ||
| profile = profile[self.config.profile_name] |
There was a problem hiding this comment.
Consider using profile.get(self.config.profile_name, {}) to avoid potential KeyError if self.config.profile_name is not in the profile dictionary.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- dbt_core_integration.py (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
dbt_core_integration.py (2)
Line range hint
1-874: Summary of changes and potential impactThe modifications in this file are focused on the
get_target_namesmethod of theDbtProjectclass. The changes improve how profiles are accessed by usingself.config.profile_nameinstead ofself.project_name. This should make the method more robust and less prone to errors.While the changes appear to be an improvement, it's important to:
- Ensure that
self.config.profile_nameis consistently set and used throughout the project where profile access is needed.- Verify that this change doesn't introduce any unintended side effects in other parts of the codebase that might rely on the previous behavior.
- Update any documentation or comments that might reference the old way of accessing profiles.
Overall, the changes seem to be a positive step towards more accurate profile handling, but careful testing and verification are recommended to ensure full compatibility with the rest of the system.
872-874: Verify consistent usage ofprofile_namevsproject_nameThe change from
self.project_nametoself.config.profile_namein theget_target_namesmethod might have implications for other parts of the code. To ensure consistency and prevent potential issues:
- Review other methods in this class and related classes to check if they should also use
self.config.profile_nameinstead ofself.project_namewhen accessing profile information.- Verify that
self.config.profile_nameis always set correctly throughout the project lifecycle.#!/bin/bash # Search for occurrences of project_name and profile_name in the codebase echo "Occurrences of project_name:" grep -rn "project_name" . echo "Occurrences of profile_name:" grep -rn "profile_name" .This will help identify any inconsistencies in how profile and project names are used throughout the codebase.
| profile = profile[self.config.profile_name] | ||
| if "outputs" in profile: | ||
| outputs = profile["outputs"] |
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent usage of profile_name detected.
The profile_name configuration is only referenced in dbt_core_integration.py:872, whereas project_name is used extensively throughout the codebase. This inconsistency may lead to configuration issues.
- Ensure that
profile_nameis properly defined and utilized whereverproject_nameis currently in use.
🔗 Analysis chain
Improved profile access in get_target_names method.
The changes in this method improve how the profile is accessed:
- It now uses
self.config.profile_nameinstead ofself.project_nameto access the correct profile. - The method checks for the presence of "outputs" in the profile before attempting to return the keys.
These changes should make the method more robust and less prone to errors when accessing profile information.
However, to ensure full compatibility, we should verify that self.config.profile_name is always set correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if profile_name is consistently set in the config
grep -rn "profile_name" . | grep -v "get_target_names"
Length of output: 1991
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
Fix
get_target_names()indbt_core_integration.pyto correctly useself.config.profile_namefor profile lookup.get_target_names()indbt_core_integration.pyto useself.config.profile_nameinstead ofself.project_namefor profile lookup.outputs.This description was created by
for 5dd6acc. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes