fix: (CDK) (manifest-only) - add ability to debug manifest-only sources / destinations
#407
Conversation
bazarnov
requested review from
maxi297,
aaronsteers,
aldogonzalez8 and
agarctfi
There was a problem hiding this comment.
Pull Request Overview
This pull request adds the ability to debug "manifest-only" sources/destinations in the CDK by introducing a dedicated debug script and providing detailed VSCode configuration instructions in the README.
- Added a new README with step-by-step instructions to configure and use the debugger.
- Introduced a debug_manifest.py file to launch the debugging session using the manifest.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| debug_manifest/README.md | Documentation for configuring the debugger with VSCode and resource placement for manifest-only connectors. |
| debug_manifest/debug_manifest.py | A new script to initiate debugging of manifest-only sources. |
📝 WalkthroughWalkthroughThe pull request introduces three new files within the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant DM as debug_manifest.py
participant YDS as YamlDeclarativeSource
participant L as launch Function
U->>DM: Execute script with command-line arguments
DM->>YDS: Create source instance from YAML configuration
DM->>L: Invoke launch(source, args)
L->>U: Execute debugging process and respond
Suggested labels
Suggested reviewers
Would you like to add any further clarifications or additional diagrams, wdyt? 🪧 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 (5)
debug_manifest/debug_manifest.py (1)
13-15: Configuration looks good, but have you considered making the path configurable?The hardcoded path works fine for the typical use case, but would it be helpful to allow users to override this path via an environment variable or command-line argument for more flexibility? wdyt?
debug_manifest/README.md (4)
51-51: Minor grammar improvement opportunity.Would this read better with a comma after "Alternatively" and correcting "swith" to "switch"?
- 8. Replace the `"python": "<PATH_TO_CDK_ENV>/bin/python"` with the correct interpreter `PATH` pointing to the `CDK env` installed from Step `7` (use `which python` to have the complete python path), to wire the CDK env to the debugger. Alternatively you can swith the default interpreter you use in your IDE. + 8. Replace the `"python": "<PATH_TO_CDK_ENV>/bin/python"` with the correct interpreter `PATH` pointing to the `CDK env` installed from Step `7` (use `which python` to have the complete python path), to wire the CDK env to the debugger. Alternatively, you can switch the default interpreter you use in your IDE.🧰 Tools
🪛 LanguageTool
[typographical] ~51-~51: Consider adding a comma after ‘Alternatively’ for more clarity.
Context: ...), to wire the CDK env to the debugger. Alternatively you can swith the default interpreter y...(RB_LY_COMMA)
54-54: Small typo in documentation.- * These resources are ignorred by `git`, in the `.gitignore`, thus should not be commited + * These resources are ignored by `git`, in the `.gitignore`, thus should not be committed
56-59: Resource setup instructions are clear, but structure could be improved.The instructions are clear, but three consecutive sentences begin with "Put the...". Would you consider restructuring them for better readability? Perhaps using a table or a different format? wdyt?
🧰 Tools
🪛 LanguageTool
[style] ~58-~58: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...red catalogfor the target source). 3. Put themanifest.yamlinside the/airbyt...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
61-61: Small typo in heading.- ## Debuging Steps + ## Debugging Steps
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
debug_manifest/.gitignore(1 hunks)debug_manifest/README.md(1 hunks)debug_manifest/debug_manifest.py(1 hunks)
🧰 Additional context used
🪛 LanguageTool
debug_manifest/README.md
[typographical] ~51-~51: Consider adding a comma after ‘Alternatively’ for more clarity.
Context: ...), to wire the CDK env to the debugger. Alternatively you can swith the default interpreter y...
(RB_LY_COMMA)
[style] ~58-~58: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...red catalogfor the target source). 3. Put themanifest.yamlinside the/airbyt...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (5)
debug_manifest/.gitignore (1)
1-5: LGTM! Well-structured .gitignore for managing resources directory.The .gitignore file is correctly configured to ignore all files in the resources directory while preserving the directory structure through .gitkeep. This approach ensures that developers can have their local configuration files without accidentally committing them, which is ideal for debugging purposes.
debug_manifest/debug_manifest.py (2)
17-23: Function implementation is clean and well-documented.The function is well-typed with appropriate annotations and has a clear docstring. The implementation is simple and directly leverages the existing CDK launch functionality.
25-29: Entry point looks good.The main block correctly initializes the source with the configuration and passes command-line arguments to the debug_manifest function.
debug_manifest/README.md (2)
14-47: VSCode configuration example is clear and well-documented.The JSON configuration example includes helpful comments for each argument and covers all the essential parameters needed for debugging manifest-only sources and destinations.
67-67: Great summary that captures the value of this feature.This concluding line effectively summarizes the benefits of this new debugging capability, highlighting how it brings parity between manifest-only sources and regular Python sources.
|
Automation ideas for the future:
Ideally, it would be very handy if the export functionality had a feature called |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
debug_manifest/README.md (5)
1-7: Clarify the introduction section?
The introduction and headings in lines 1–7 provide a clear context for the debugging setup. Would you consider adding a brief "Overview" or "Prerequisites" sub-section for developers who might be new to the CDK? wdyt?
14-47: Review the JSON snippet details?
The providedlaunch.jsonconfiguration is comprehensive and illustrative. Note that while standard JSON does not support comments, VSCode’s configuration uses JSONC which permits them. Would you consider adding a brief note (perhaps in a comment preceding the block) clarifying that the inline comments are for documentation purposes only? wdyt?
49-51: Improve readability in the interpreter instructions.
The instructions after the JSON snippet are clear. However, on line 51, consider adding a comma after “Alternatively” for better clarity. For example:-...to wire the CDK env to the debugger. Alternatively you can switch the default interpreter you use in your IDE. +...to wire the CDK env to the debugger. Alternatively, you can switch the default interpreter you use in your IDE.Would you be open to this change? wdyt?
🧰 Tools
🪛 LanguageTool
[typographical] ~51-~51: Consider adding a comma after ‘Alternatively’ for more clarity.
Context: ...), to wire the CDK env to the debugger. Alternatively you can switch the default interpreter ...(RB_LY_COMMA)
52-57: Enhance details for resource setup?
The steps for setting up the necessary resources (lines 52–57) are succinct and clear. Would you consider adding a brief description of what each of the resource files (e.g.,config.json,catalog.json,manifest.yaml, and optionallystate.json) should contain? This might help developers better prepare the required files. wdyt?
59-63: Clarify the "Iterate" instruction.
The debugging steps are generally clear. However, the instruction on line 62—"Iterate over the2and3"—might be a bit ambiguous. Would you consider rephrasing it to "Repeat steps 2 and 3" to improve clarity? wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
debug_manifest/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
debug_manifest/README.md
[typographical] ~51-~51: Consider adding a comma after ‘Alternatively’ for more clarity.
Context: ...), to wire the CDK env to the debugger. Alternatively you can switch the default interpreter ...
(RB_LY_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Pytest (Fast)
🔇 Additional comments (2)
debug_manifest/README.md (2)
8-12: Great step-by-step instructions!
The steps outlining the process to create and configure thelaunch.jsonfile are well organized and easy to follow. Nice job!
64-64: Effective final summary!
The concluding note effectively communicates that users can now runmanifest-onlysources with full command support and debugging options. This is clear and concise.
What
Frequently, we had cases when we need/have to
debugamanifest-onlysource/destination, but we don't have a fast and relatively easy way of doing that to reproduce the issue or introduce new features to thedeclarative_component_schema, etc.How
manifest-onlysources and destinations required creating a copy of thesource-declarative-manifest,modifying thepyproject.toml, installing an environment, and ensuring compatibility with versioning of the manifest and other related elements.manifest-onlysources and destinations by providing only the necessary components, such asconfig, catalog, stateand themanifestitself.breakpointsand gain a comprehensive understanding of the runtime state to identify and resolve any issues effectively.README.mdfor more information.Summary by CodeRabbit
New Features
Documentation
Chores