-
Notifications
You must be signed in to change notification settings - Fork 16.5k
chore(trino): Add process_text for additional running state info #36909
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
base: master
Are you sure you want to change the base?
chore(trino): Add process_text for additional running state info #36909
Conversation
Code Review Agent Run #0df76fActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Nitpicks 🔍
|
|
CodeAnt AI finished reviewing your PR. |
574fb14 to
3115824
Compare
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #36909 +/- ##
==========================================
+ Coverage 60.48% 68.16% +7.67%
==========================================
Files 1931 639 -1292
Lines 76236 47665 -28571
Branches 8568 5206 -3362
==========================================
- Hits 46114 32492 -13622
+ Misses 28017 13893 -14124
+ Partials 2105 1280 -825
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Code Review Agent Run #bc3755
Actionable Suggestions - 2
-
tests/unit_tests/db_engine_specs/test_trino.py - 2
- Weak test assertion for progress_text mapping · Line 1260-1260
- Weak test assertion for progress_text mapping · Line 1304-1304
Additional Suggestions - 1
-
superset/db_engine_specs/trino.py - 1
-
Missing Type Hints · Line 229-248The project standards require type hints for all new Python code. Please add `: bool = False` to the `needs_commit` assignment and `: str` to the `progress_text` assignment to improve maintainability and enable MyPy validation.
-
Review Details
-
Files reviewed - 2 · Commit Range:
3115824..3115824- superset/db_engine_specs/trino.py
- tests/unit_tests/db_engine_specs/test_trino.py
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
| TrinoEngineSpec.handle_cursor(cursor=cursor_mock, query=query) | ||
|
|
||
| # Check that progress_text was set to "Scheduled" for PLANNING state | ||
| assert query.extra.get("progress_text") is not None |
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.
The assertion here is too weak—it only checks that progress_text is set, but doesn't verify the correct mapped value for PLANNING state. Since the code sets it to 'Scheduled' for PLANNING, the test should assert equality to catch mapping errors.
Code suggestion
Check the AI-generated fix before applying
--- a/tests/unit_tests/db_engine_specs/test_trino.py
+++ b/tests/unit_tests/db_engine_specs/test_trino.py
@@ -1259,7 +1259,7 @@ def test_handle_cursor_sets_progress_text_for_planning_state(
# Check that progress_text was set to "Scheduled" for PLANNING state
- assert query.extra.get("progress_text") is not None
+ assert query.extra.get("progress_text") == "Scheduled"
Code Review Run #bc3755
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
| TrinoEngineSpec.handle_cursor(cursor=cursor_mock, query=query) | ||
|
|
||
| # Check that progress_text was set | ||
| assert query.extra.get("progress_text") is not None |
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.
Similar to the PLANNING test, this assertion is too weak—it doesn't verify the correct 'Queued' value for QUEUED state, potentially missing mapping bugs.
Code suggestion
Check the AI-generated fix before applying
--- a/tests/unit_tests/db_engine_specs/test_trino.py
+++ b/tests/unit_tests/db_engine_specs/test_trino.py
@@ -1303,7 +1303,7 @@ def test_handle_cursor_sets_progress_text_for_queued_state(
# Check that progress_text was set
- assert query.extra.get("progress_text") is not None
+ assert query.extra.get("progress_text") == "Queued"
Code Review Run #bc3755
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
User description
SUMMARY
In addition to the existing query states, Trino provides additional statuses—
queuedandplanning—during the running state. This commit includes this information inprogress_textto provide more detailed status updates about the running state on the frontend.TESTING INSTRUCTIONS
specs
ADDITIONAL INFORMATION
CodeAnt-AI Description
Show Trino queued/planning status in query progress and return readable progress text
What Changed
Impact
✅ Clearer Trino running-state messages in SQL Lab✅ Progress text visible in query results API✅ Fewer redundant database updates during query polling💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.