tests: add debug option for entrypoint wrapper #618
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request adds a debug parameter to the EntrypointWrapper.read method to enable appending the "--debug" flag during its execution for debugging purposes.
- Adds a new parameter "debug" to the method signature.
- Conditionally appends the "--debug" option to the args list if debug is true.
Comments suppressed due to low confidence (1)
airbyte_cdk/test/entrypoint_wrapper.py:376
- Consider updating the docstring of the read method to document the newly added 'debug' parameter and its intended effect.
debug: bool = False,
📝 WalkthroughWalkthroughA new optional Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant EntrypointWrapper
participant _run_command
Caller->>EntrypointWrapper: read(..., debug=True/False)
alt debug is True
EntrypointWrapper->>EntrypointWrapper: Append "--debug" to args
end
EntrypointWrapper->>_run_command: Call with args (possibly with "--debug")
_run_command-->>EntrypointWrapper: Return result
EntrypointWrapper-->>Caller: Return EntrypointOutput
✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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_cdk/test/entrypoint_wrapper.py (1)
378-383: Should we update the docstring to mention the new debug parameter?The docstring currently doesn't document the
debugparameter. Would it be helpful to add a brief description of what it does, something like:param debug: Whether to run the read command with debug logging enabled? wdyt?""" config and state must be json serializable :param expected_outcome: By default if there is an uncaught exception, the exception will be printed out. If this is expected, please provide `expected_outcome=ExpectedOutcome.EXPECT_FAILURE` so that the test output logs are cleaner. + :param debug: Whether to run the read command with debug logging enabled. """
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
airbyte_cdk/test/entrypoint_wrapper.py(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Check: source-shopify
🔇 Additional comments (3)
airbyte_cdk/test/entrypoint_wrapper.py (3)
376-376: Nice addition of the debug parameter!The keyword-only parameter with a sensible default looks good. This maintains backward compatibility while adding the new functionality. The type hint and placement are spot-on too!
398-399: Clean and straightforward implementation!The conditional logic to append "--debug" is simple and effective. This should work perfectly for enabling debug mode during testing.
362-362: Potential inconsistency with the discover function?I noticed that the
discoverfunction on line 362 always includes--debugin its args, while your newreadfunction only includes it conditionally. Should we consider making thediscoverfunction consistent by adding a similardebugparameter? Or is this intentional because discovery operations always need debug info? wdyt?Let me check if there are other functions that might benefit from similar debug parameter consistency:
#!/bin/bash # Description: Check for other functions that might use --debug flag # Expected: Find other functions that hardcode --debug to assess consistency rg -A 10 -B 5 '"--debug"' --type py
What
debugparameter toEntrypointWrapper.readmethod, when set to true, will append the--debugoption to the list of args passed to the read operation.Why
Summary by CodeRabbit