fix: give logs a second to flush #660
Conversation
There was a problem hiding this comment.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
airbyte/_connector_base.py:329
- Using sleep(1) may not guarantee that logs are flushed in all environments; consider using an explicit log flush method or making the delay configurable to better ensure logs are output before error raising.
sleep(1)
📝 WalkthroughWalkthroughThis change introduces a one-second pause after logging a failed connector check in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Connector
participant Logger
User->>Connector: check()
Connector->>Connector: Perform check logic
alt Check fails
Connector->>Logger: Log failure
Connector->>Connector: sleep(1)
Connector->>User: Raise AirbyteConnectorCheckFailedError
else Check succeeds
Connector->>User: Return success
end
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
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:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 0
🧹 Nitpick comments (1)
airbyte/_connector_base.py (1)
328-329: Nice touch adding a pause to help logs flush!Adding
sleep(1)after logging and before raising the exception is a pragmatic way to help ensure logs are visible before the process potentially exits. Would you consider either making the delay configurable (maybe via an optional parameter or class attribute), or adding a brief comment explaining why 1 second was chosen? This might help future maintainers understand the rationale and adjust if needed. Wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
airbyte/_connector_base.py(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Pytest (All, Python 3.11, Windows)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (No Creds)
- GitHub Check: Pytest (All, Python 3.10, Windows)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
🔇 Additional comments (1)
airbyte/_connector_base.py (1)
9-9: Import ofsleepfromtimelooks good!This import is minimal and directly supports the new logic. No concerns here.
Summary by CodeRabbit