-
Notifications
You must be signed in to change notification settings - Fork 0
Support set type in query builder operator handling
#151
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
WalkthroughThe RQL query builder's list-operator value handling now accepts sets in addition to lists and tuples. Type checking for constants.LIST operators was expanded to recognize set types, causing set values to be joined with commas during encoding, matching the existing behavior for lists and tuples. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
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: Set support inrql_encodelooks good; consider docs and ordering implications.The updated type check to accept
setalongsidelist/tupleforconstants.LISToperators matches the PR goal and is functionally correct. A couple of small follow‑ups you may want to consider:
- The
rql_encodedocstring and Args description still only mention “list/tuple”. It would be clearer to explicitly document thatsetis also accepted for list operators.- Joining a
setwith",".join(value)makes the encoded string’s element order non‑deterministic, which is fine semantically forin/out, but might surprise callers or tests that assert on the exact string. If deterministic output matters, you could optionally normalize sets before joining (e.g., by sorting or documenting that order is undefined for sets).No blocking issues here; these are minor polish/behavior‑clarity suggestions around the new
setsupport.
📜 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
|



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