chore(ibis): return v3 error instead of v2 error#1296
Conversation
|
Warning Rate limit exceeded@goldmedal has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
WalkthroughIntroduces inner try/except wrappers around all v2 fallbacks in the v3 connector. When a v2 fallback raises, the original v3 exception is re-raised. Applied to query, dry_plan, dry_plan_for_data_source, and model_substitute. No changes to signatures, parameters, or success-path logic. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant V3 as V3 Router/Connector
participant S as Service Logic
participant V2 as V2 Connector (fallback)
C->>V3: Request (e.g., query)
V3->>S: Execute v3 path
alt v3 succeeds
S-->>V3: v3 result
V3-->>C: 200 OK with result
else v3 raises e
Note over V3: Fallback to v2
V3->>V2: Attempt v2 call (wrapped try/except)
alt v2 succeeds
V2-->>V3: v2 result
V3-->>C: 200 OK with v2 result
else v2 raises err_v2
Note over V3: Re-raise original v3 error e (not err_v2)
V3-->>C: Error (from v3)
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
ibis-server/app/routers/v3/connector.py (1)
370-382: Consistency gap: validate fallback still surfaces v2 errors.If the goal is “always return v3 error when v2 fallback fails,” consider aligning
validatewith the same pattern.Proposed change:
- headers = append_fallback_context(headers, span) - return await v2.connector.validate( - data_source=data_source, - rule_name=rule_name, - dto=dto, - java_engine_connector=java_engine_connector, - headers=headers, - is_fallback=True, - ) + headers = append_fallback_context(headers, span) + try: + return await v2.connector.validate( + data_source=data_source, + rule_name=rule_name, + dto=dto, + java_engine_connector=java_engine_connector, + headers=headers, + is_fallback=True, + ) + except Exception as ve: + logger.debug( + "v2 fallback also failed for v3 validate; returning original v3 error: {}", + str(ve), + ) + raise e from None
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ibis-server/app/routers/v3/connector.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
ibis-server/app/routers/v3/connector.py (1)
ibis-server/app/routers/v2/connector.py (4)
query(65-202)dry_plan(312-329)dry_plan_for_data_source(337-358)model_substitute(366-398)
⏰ 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: ci
2 participants
If the SQL fails in the v2 engine, we should return the v3 error instead of the v2 error. It's helpful for us to focus on fixing the v3 SQL syntax.
Summary by CodeRabbit