[ROB-2094] added supabase conectivity log and instructions on failure #1949
Conversation
|
Warning Rate limit exceeded@Avi-Robusta has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 11 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 selected for processing (1)
WalkthroughAdds a new Changes
Sequence DiagramsequenceDiagram
participant Caller
participant Config as construct_new_sinks\n(model/config.py)
participant ConfigLoader as __reload_playbook_packages\n(runner/config_loader.py)
participant DAL as supabase_dal.sign_in
participant SupabaseSDK as Supabase SDK
Caller->>Config: request sink creation
Config->>DAL: call sign_in()
DAL->>SupabaseSDK: authenticate
alt DNS / connectivity error
SupabaseSDK-->>DAL: socket.gaierror / DNS error
DAL->>DAL: detect DNS-like message
DAL-->>Config: raise SupabaseDnsException(original, url)
Config->>Config: log error (exc_info=False)
Config-->>Caller: mark sink error / skip sink
else Other exception
SupabaseSDK-->>DAL: other exception
DAL-->>Config: propagate original exception
Config->>Config: log error (exc_info=True)
Config-->>Caller: handle/raise per existing flow
else Success
SupabaseSDK-->>DAL: auth success
DAL-->>Config: return credentials
Config-->>Caller: continue normal flow
end
Note right of Caller: Later, during playbook reload
Caller->>ConfigLoader: trigger reload
ConfigLoader->>ConfigLoader: try reload packages
alt SupabaseDnsException during reload
ConfigLoader-->>ConfigLoader: log error (exc_info=False)
else Other exception
ConfigLoader-->>ConfigLoader: log error (exc_info=True)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
![coderabbitai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/robusta/core/sinks/robusta/dal/supabase_dal.py (1)
544-566: Good DNS error handling with actionable user guidance.The implementation correctly detects DNS-related connectivity errors and provides users with clear troubleshooting steps. The DNS detection heuristic covers common error messages, and the exception chaining preserves the original context.
Optional: Address static analysis hint
Consider moving the return statement to an
elseblock as suggested by Ruff (TRY300) for slightly clearer control flow:def sign_in(self) -> str: logging.info("Supabase dal login") try: res = self.client.auth.sign_in_with_password({"email": self.email, "password": self.password}) self.client.auth.set_session(res.session.access_token, res.session.refresh_token) self.client.postgrest.auth(res.session.access_token) - return res.user.id except Exception as e: # Check if this is a DNS-related error error_msg = str(e).lower() if any(dns_indicator in error_msg for dns_indicator in [ "temporary failure in name resolution", "name resolution", "dns", "name or service not known", "nodename nor servname provided" ]): message = ( f"\n{e.__class__.__name__}: {e}\nCannot connect to Robusta SaaS <{self.url}>. " f"\nThis is often due to DNS issues or Firewall policies. " f"\nPlease run the following command in your cluster and verify it does not print 'Could not resolve host': " f"\ncurl -I {self.url}\n" ) raise SupabaseDnsException(message) from e raise + else: + return res.user.id
📜 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(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/robusta/core/sinks/robusta/dal/supabase_dal.py (1)
src/robusta/core/exceptions.py (1)
SupabaseDnsException(19-25)
🪛 Ruff (0.14.2)
src/robusta/core/sinks/robusta/dal/supabase_dal.py
548-548: Consider moving this statement to an else block
(TRY300)
⏰ 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 (1)
src/robusta/core/sinks/robusta/dal/supabase_dal.py (1)
20-20: LGTM!The import of
SupabaseDnsExceptionis correctly placed and supports the DNS error handling added to thesign_inmethod.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/robusta/runner/config_loader.py (1)
275-281: Good approach to reduce log noise for DNS errors.The conditional traceback suppression logic is sound: DNS-related exceptions provide user-friendly messages with troubleshooting steps, making stack traces unnecessary, while other exceptions retain full debugging context.
Minor polish: remove trailing whitespace on Line 277 and consider rephrasing the comment for clarity:
- except Exception as e: - print_exception = not isinstance(e, SupabaseDnsException) # Don't show trace for Robusta SaaS DNS-related exception to avoid hiding the error in noisy logs - + except Exception as e: + # Suppress traceback for DNS exceptions; their custom message is more helpful than a stack trace + print_exception = not isinstance(e, SupabaseDnsException) + logging.error(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/robusta/core/exceptions.py(1 hunks)src/robusta/core/sinks/robusta/dal/supabase_dal.py(2 hunks)src/robusta/runner/config_loader.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/robusta/runner/config_loader.py (1)
src/robusta/core/exceptions.py (1)
SupabaseDnsException(19-33)
src/robusta/core/sinks/robusta/dal/supabase_dal.py (1)
src/robusta/core/exceptions.py (1)
SupabaseDnsException(19-33)
🪛 Ruff (0.14.2)
src/robusta/runner/config_loader.py
275-275: Do not catch blind exception: Exception
(BLE001)
src/robusta/core/sinks/robusta/dal/supabase_dal.py
548-548: Consider moving this statement to an else block
(TRY300)
⏰ 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 (4)
src/robusta/runner/config_loader.py (1)
46-46: LGTM!The import of
SupabaseDnsExceptionis correctly placed and enables DNS-specific error handling.src/robusta/core/exceptions.py (1)
19-33: Excellent custom exception implementation!The
SupabaseDnsExceptionclass provides clear, actionable error messages for DNS/connectivity failures. The message format includes:
- The original error details
- The problematic URL
- Troubleshooting guidance with a concrete curl command
Storing
original_exceptionandurlas attributes enables programmatic access if needed downstream.src/robusta/core/sinks/robusta/dal/supabase_dal.py (2)
20-20: LGTM!The import enables DNS-specific exception handling in the sign-in flow.
544-561: DNS error detection is comprehensive and properly implemented.Verification confirms the exception handling is sound:
- Keywords cover common DNS failures across Python and glibc: "temporary failure in name resolution", "name resolution", "dns", "name or service not known", "nodename nor servname provided"
- These patterns match standard socket.gaierror messages across Python versions
- SupabaseDnsException provides actionable troubleshooting guidance with curl command
- Exception chaining via
raise ... from ecorrectly preserves the original error context- No competing DNS error patterns elsewhere in the codebase, ensuring consistency
The generic exception catching with message filtering is appropriate for the Supabase client library, which wraps network errors. No changes needed.
on failure now prints error like