docs: pair session output: AJ + Brian Lai #212
Conversation
📝 WalkthroughWalkthroughThis pull request focuses on documentation updates for the Airbyte CDK, primarily introducing deprecation notices for several cursor and stream-related classes while adding documentation for key declarative source classes. The changes aim to guide developers towards newer, recommended implementations and provide clearer documentation about the available interfaces for building declarative sources. Changes
Possibly related PRs
Suggested reviewers
Hey there! Quick question - would you like me to elaborate on any of these changes? The deprecation notices seem pretty straightforward, but I'm happy to dive deeper if you'd like. Wdyt? 🤔 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
🔭 Outside diff range comments (1)
pyproject.toml (1)
Line range hint
1-200: Update poetry.lock fileThe pipeline is failing because the lock file needs to be updated. Please run:
poetry lock --no-update🧰 Tools
🪛 GitHub Actions: Pytest (Fast)
[error] pyproject.toml changed significantly since poetry.lock was last generated. Run
poetry lock [--no-update]to fix the lock file.
🧹 Nitpick comments (5)
airbyte_cdk/sources/declarative/incremental/global_substream_cursor.py (1)
Line range hint
75-85: Would you like to add a configuration example to the docstring?The docstring is comprehensive, but users might benefit from seeing a concrete example of how to configure the
global_substream_cursor. What do you think about adding a YAML configuration example? wdyt?- When using the `incremental_dependency` option, the sync will progress through parent records, preventing the sync from getting infinitely stuck. However, it is crucial to understand the requirements for both the `global_substream_cursor` and `incremental_dependency` options to avoid data loss. + + Example configuration: + ```yaml + incremental_sync: + type: "GlobalSubstreamCursor" + stream_cursor: + type: "DatetimeBasedCursor" + datetime_format: "%Y-%m-%dT%H:%M:%SZ" + cursor_field: "updated_at" + partition_router: + type: "SubstreamPartitionRouter" + parent_stream_configs: + - stream: "parent_stream" + parent_key: "id" + partition_field: "parent_id" + ``` """pyproject.toml (1)
91-91: Consider pinning the pdoc versionThe pdoc version update looks good, but using
^15.0.1allows for potentially breaking changes. Would you consider pinning to a specific version for better reproducibility? wdyt?-pdoc = "^15.0.1" +pdoc = "15.0.1"🧰 Tools
🪛 GitHub Actions: Pytest (Fast)
[error] pyproject.toml changed significantly since poetry.lock was last generated. Run
poetry lock [--no-update]to fix the lock file.airbyte_cdk/sources/declarative/incremental/per_partition_with_global.py (1)
22-26: Consider enhancing the deprecation notice with migration guidance.The deprecation notice is clear, but developers might benefit from more detailed migration steps. What do you think about adding a brief example of how to migrate from
PerPartitionWithGlobalCursortoConcurrentCursor? Something like:# Before cursor = PerPartitionWithGlobalCursor(...) # After cursor = ConcurrentCursor(...)airbyte_cdk/__init__.py (1)
19-27: The new documentation section is helpful! Want to make it even better? 🚀Great addition of the key classes section! To make it even more valuable for developers, what do you think about:
- Adding code examples for each class?
- Including links to detailed API documentation?
- Mentioning common use cases or scenarios for each class?
airbyte_cdk/sources/declarative/incremental/per_partition_cursor.py (1)
29-33: Consider adding migration guidance for consistency.The deprecation notice is clear, but for consistency with other files and to help developers, what do you think about adding migration guidance similar to what we suggested for
PerPartitionWithGlobalCursor? This would help maintain a consistent documentation style across the codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
airbyte_cdk/__init__.py(1 hunks)airbyte_cdk/sources/declarative/declarative_stream.py(2 hunks)airbyte_cdk/sources/declarative/incremental/datetime_based_cursor.py(2 hunks)airbyte_cdk/sources/declarative/incremental/global_substream_cursor.py(2 hunks)airbyte_cdk/sources/declarative/incremental/per_partition_cursor.py(2 hunks)airbyte_cdk/sources/declarative/incremental/per_partition_with_global.py(2 hunks)airbyte_cdk/sources/declarative/stream_slicers/declarative_partition_generator.py(0 hunks)pyproject.toml(2 hunks)
💤 Files with no reviewable changes (1)
- airbyte_cdk/sources/declarative/stream_slicers/declarative_partition_generator.py
✅ Files skipped from review due to trivial changes (1)
- airbyte_cdk/sources/declarative/incremental/datetime_based_cursor.py
🧰 Additional context used
🪛 GitHub Actions: Pytest (Fast)
pyproject.toml
[error] pyproject.toml changed significantly since poetry.lock was last generated. Run poetry lock [--no-update] to fix the lock file.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Check: 'source-pokeapi' (skip=false)
- GitHub Check: Check: 'source-the-guardian-api' (skip=false)
- GitHub Check: Check: 'source-shopify' (skip=false)
- GitHub Check: Check: 'source-hardcoded-records' (skip=false)
🔇 Additional comments (4)
airbyte_cdk/sources/declarative/incremental/global_substream_cursor.py (2)
9-10: LGTM! Clear deprecation notice.The deprecation notice and docstring update effectively communicate the transition to
ConcurrentCursor. The original documentation is preserved while clearly marking the deprecation status.Also applies to: 70-70, 73-74
Line range hint
258-261: Consider handling timer edge casesThe lookback window calculation assumes the timer was started. While the
start_slices_generationmethod is called instream_slices, there might be edge cases where the timer wasn't started. Should we add a guard clause infinish()to handle this case more gracefully? wdyt?pyproject.toml (1)
54-54: LGTM! Required dependency for deprecation notices.The addition of
typing_extensionsis necessary for the@deprecateddecorator usage.🧰 Tools
🪛 GitHub Actions: Pytest (Fast)
[error] pyproject.toml changed significantly since poetry.lock was last generated. Run
poetry lock [--no-update]to fix the lock file.airbyte_cdk/sources/declarative/declarative_stream.py (1)
33-39: Great job on providing context about future plans! 👍The deprecation notice effectively communicates both the immediate action (use
DefaultStream) and the longer-term context (future refactoring plans). This helps developers understand the bigger picture.
|
@brianjlai - Do you mind taking a look through the code changes? If anything is wrong or not as precise as it can be, you can use "suggest changes" and I'll apply those directly. |
|
/autofix
|
Adds a new section to the API ref top-level page:
Marks some classes as deprecated, e.g.:
Summary by CodeRabbit
Release Notes
Documentation
Dependencies
typing_extensionslibrary.pdocdevelopment dependency to version 15.0.1.Deprecations
DeclarativeStream,DatetimeBasedCursor,GlobalSubstreamCursor,PerPartitionCursor, andPerPartitionWithGlobalCursoras deprecated.DefaultStreamandConcurrentCursoras replacements.