-
Notifications
You must be signed in to change notification settings - Fork 204
Correctly shorten versioned model names #2043
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Correctly shorten versioned model names #2043
Conversation
|
👋 @Ryosuke839 |
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
elementary/utils/models.py(1 hunks)tests/unit/utils/test_models.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/unit/utils/test_models.py (1)
elementary/utils/models.py (1)
get_shortened_model_name(1-6)
🪛 Flake8 (7.3.0)
tests/unit/utils/test_models.py
[error] 19-19: redefinition of unused 'test_shorten_1_part' from line 9
(F811)
🪛 Ruff (0.14.2)
tests/unit/utils/test_models.py
19-19: Redefinition of unused test_shorten_1_part from line 9
(F811)
🔇 Additional comments (1)
elementary/utils/models.py (1)
5-6: Logic correctly implements versioned model name shortening with full backward compatibility.Verification confirms the implementation works as intended:
- All unit tests pass (4 test cases covering None, 1-part, 3-part, and 4-part model names)
- Usage in
elementary/monitor/alerts/alerts_groups/grouped_by_table.pyis compatible with the updated behavior- No breaking changes to existing code
|
Hey @Ryosuke839 ! Thank you for your contribution, appreciate it. |
|
@Ryosuke839 Thanks again for the contribution! |
Elementary shortens model name in Slack alerts. However, it will only show model version like
v2when tests were against versioned models https://docs.getdbt.com/docs/mesh/govern/model-versions.(Note: Slack alert message is generated in
elementary/elementary/monitor/alerts/alerts_groups/grouped_by_table.py
Lines 24 to 40 in ba986eb
This PR changes shortening logic so that shortened model name will consist of both model base name and model version like
model.v2.Summary by CodeRabbit
Bug Fixes
Tests