-
Notifications
You must be signed in to change notification settings - Fork 50
chore: update destination example script configuration to use GitHub and state cache #659
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: update destination example script configuration to use GitHub and state cache #659
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
📝 WalkthroughWalkthroughThe changes update the example script to use the "source-github" connector instead of "source-faker", requiring a repository list and GitHub credentials. The script now explicitly checks the source and selects the "issues" stream. A new function is introduced to create a local DuckDB cache, which is then used as the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Source (GitHub)
participant Cache (DuckDB)
participant Destination
User->>Source (GitHub): Initialize with repo list and credentials
User->>Source (GitHub): check()
User->>Source (GitHub): select_stream("issues")
User->>Cache (DuckDB): get_cache()
User->>Destination: write(source, state_cache=Cache, cache=False)
Destination->>Cache (DuckDB): Use for state caching
Destination->>Source (GitHub): Read issues stream
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
examples/run_sync_to_destination_wo_cache.py (2)
20-35
: The GitHub configuration looks good, but is there a fail-safe needed?The switch from Faker to GitHub source looks great, and I like the explicit stream selection. The assertion on line 22 will prevent the script from running with an empty token, which is good.
One small thought - should we add a friendlier error message if the token is missing or invalid? Something like
assert str(github_pat), "GitHub token is required. Set the GITHUB_PERSONAL_ACCESS_TOKEN secret."
might help users debug, wdyt?
63-67
: Interesting approach with cache parametersI notice you're setting
cache=False
while also providing astate_cache
. This seems intentional based on the PR title (which mentions "wo_cache" while still using state cache).Could we add a brief comment explaining this approach for clarity? Something like
# Using state_cache for persistence while disabling in-memory cache
might help users understand the distinction, wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/run_sync_to_destination_wo_cache.py
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`examples/*`: These scripts are intended for demonstration purposes only. They are not meant to represent production code. For these scripts, you should prefer brevity over robust ...
examples/*
: These scripts are intended for demonstration purposes only. They are not meant to represent production code. For these scripts, you should prefer brevity over robust error handling and handling of edge cases. These are demos - which should be as simple as possible to handle the 'blessed' use cases.
examples/run_sync_to_destination_wo_cache.py
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Validate PR title
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.11, Windows)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Windows)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (No Creds)
🔇 Additional comments (2)
examples/run_sync_to_destination_wo_cache.py (2)
31-35
: Nice improvement with explicit stream selection!I like how you've changed from specifying streams in the constructor to explicitly selecting them after creating the source. This approach makes the code more flexible and easier to understand.
38-41
: Good addition of the cache functionCreating a dedicated function for getting the cache instance is a clean approach. This makes the code more modular and easier to maintain.
For a demo script, this level of simplicity is appropriate since we're focusing on the "blessed" use case.
Summary by CodeRabbit