-
Notifications
You must be signed in to change notification settings - Fork 0
Fix handle non-string elements in iterable for query builder #153
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
Conversation
WalkthroughA bug fix in the RQL query builder that improves LIST operator handling by converting all iterable elements to strings before joining them, preventing errors when non-string elements are present. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (1)
mpt_api_client/rql/query_builder.py (1)
107-112: Keep list element encoding consistent withquery_value_strThe change fixes the
joinfailure for non-string elements, butstr(el)bypasses the existing encoding rules (e.g., booleans becomeTrue/Falseinstead oftrue/false). To keep scalar and list encodings consistent, consider delegating toquery_value_str:- if op in constants.LIST and isinstance(value, list | tuple | set): - return ",".join(str(el) for el in value) + if op in constants.LIST and isinstance(value, list | tuple | set): + return ",".join(query_value_str(el) for el in value)If sets are commonly used here, you may also want to normalize ordering (e.g.,
sorted(...)) for deterministic output, but that’s optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mpt_api_client/rql/query_builder.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
|
Is there a jira task for this? |
|
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.
I miss the test and #153 (comment)



Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.