Patch pre_select supabase #1950
Conversation
WalkthroughA module-level monkey-patch is applied to Supabase's request builder Changes
Sequence Diagram(s)sequenceDiagram
participant Module as supabase_dal.py (module import)
participant Builder as supabase_request_builder
participant Caller as Any code calling pre_select
rect #f8f9fa
Module->>Builder: import builder\nimport QueryArgs
Module->>Builder: wrap pre_select with normalizer
Note right of Module: logs "patched pre_select"
end
Caller->>Builder: pre_select(...)
alt patched flow
Builder->>Builder: original pre_select(...)
Builder-->>Builder: returns QueryArgs (maybe json falsy)
Builder->>Builder: normalizer: if not json -> set json = None
Builder-->>Caller: normalized QueryArgs
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/robusta/core/sinks/robusta/dal/supabase_dal.py (1)
61-69: Make the monkey patch idempotentIf this module gets re-imported (for example in tests that call
importlib.reload), we will wrap the already-patchedpre_selectagain, stacking extra call layers. It still works, but each reload adds another wrapper, making debugging and profiling noisy. Please gate the patch so it only runs once.-logging.info("Patching supabase_request_builder.pre_select") -original_pre_select = supabase_request_builder.pre_select -def pre_select_patched(*args, **kwargs): - args: QueryArgs = original_pre_select(*args, **kwargs) - if not args.json: - args = QueryArgs(args.method, args.params, args.headers, None) - - return args -supabase_request_builder.pre_select = pre_select_patched +if not getattr(supabase_request_builder.pre_select, "_robusta_patch", False): + logging.info("Patching supabase_request_builder.pre_select") + original_pre_select = supabase_request_builder.pre_select + + def pre_select_patched(*args, **kwargs): + args: QueryArgs = original_pre_select(*args, **kwargs) + if not args.json: + args = QueryArgs(args.method, args.params, args.headers, None) + return args + + pre_select_patched._robusta_patch = True + supabase_request_builder.pre_select = pre_select_patched </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 060705b8917306ae9aca725499e7731698e280c0 and a15d83c1ebac61fc9bf48158bf91f8e1c35247d2. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `src/robusta/core/sinks/robusta/dal/supabase_dal.py` (3 hunks) </details> <details> <summary>⏰ 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). (3)</summary> * GitHub Check: run_tests * GitHub Check: run_tests * GitHub Check: Deploy docs </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/robusta/core/sinks/robusta/dal/supabase_dal.py (1)
61-61: Consider log level for production environments.The INFO-level log will appear in all production logs. If this patch becomes stable and well-understood, consider lowering to DEBUG to reduce log verbosity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/robusta/core/sinks/robusta/dal/supabase_dal.py(3 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). (2)
- GitHub Check: run_tests
- GitHub Check: run_tests
🔇 Additional comments (3)
src/robusta/core/sinks/robusta/dal/supabase_dal.py (3)
13-13: LGTM!The
QueryArgsimport is necessary for the monkey-patch implementation and is imported from a public module.
63-68: Patch logic appears correct, pending constructor verification.The normalization logic correctly handles falsy
jsonvalues by explicitly setting them toNone. This preservesmethod,params, andheaderswhile normalizing thejsonfield.However, ensure that:
- The
QueryArgsconstructor signature matches this call (4 positional arguments)- There are no other fields in
QueryArgsthat need to be preserved
40-40: Accessing private postgrest module risks breakage with library updates.The import from
postgrest._syncand monkey-patching ofpre_selectat lines 40 and 62–69 rely on internal implementation details not exposed in postgrest-py's public API. The_syncmodule andpre_selectare internal, non-public helpers—documented public alternatives are theSyncPostgrestClientinterfaces (e.g.,.select(),.execute()).With postgrest 0.16.8 still actively developed, these internal details could change in minor or patch releases, breaking this workaround. Consider:
- Reporting this limitation to postgrest-py maintainers to expose
pre_selectpublicly- Filing a GitHub issue documenting the JSON serialization issue this patch addresses
- Tracking this as technical debt until a stable public API alternative is available
https://github.com/robusta-dev/relay/pull/307