Feat: Upgrade to Pydantic 2.0 and CDK 2.0#291
Conversation
|
Warning Rate limit exceededAaron ("AJ") Steers (@aaronsteers) has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 22 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughWalkthroughThe updates introduce various improvements and adjustments across multiple files, including library version upgrades, enhanced Pydantic integration for validation in secrets handling, and refined processing of documents and state management. Key changes simplify source fetching and state writing, align type annotations, add new sources, and enhance encoding specifications. These modifications increase compatibility, maintainability, and functionality, particularly focusing on making the codebase more efficient and compatible with the latest dependencies. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
CodeRabbit (@coderabbitai) review |
Actions performedReview triggered.
|
![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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
Files selected for processing (3)
- examples/run_pokeapi.py (1 hunks)
- pyproject.toml (3 hunks)
- tests/integration_tests/test_all_cache_types.py (4 hunks)
Additional comments not posted (9)
examples/run_pokeapi.py (2)
17-20: LGTM! Verify the usage of the new parameter.The usage of
get_sourcehas been updated correctly. Ensure that the new parametersource_manifest=Trueis correctly used.<details> <summary>Verification successful</summary> --- <!-- [monologue] [task review_file_1/3 lines_17-20] Review the usage of `get_source`. [observations] The `source_manifest=True` parameter is used in multiple files: - `examples/run_pokeapi.py` - `tests/unit_tests/test_lowcode_connectors.py` - `examples/run_downloadable_yaml_source.py` - `tests/integration_tests/test_all_cache_types.py` - `tests/integration_tests/test_lowcode_connectors.py` [analysis] The presence of the `source_manifest=True` parameter in multiple test files suggests that its usage is being tested adequately. Additionally, its usage in example files indicates that it is being utilized in practical scenarios. [actions] Confirm that the parameter is used correctly in the context of `get_source` function calls. --> **LGTM! Verify the usage of the new parameter.** The `source_manifest=True` parameter is used correctly in multiple files, including examples and tests. This indicates adequate testing and practical usage. - `examples/run_pokeapi.py` - `tests/unit_tests/test_lowcode_connectors.py` - `examples/run_downloadable_yaml_source.py` - `tests/integration_tests/test_all_cache_types.py` - `tests/integration_tests/test_lowcode_connectors.py` </details> <details> <summary>Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Verify the usage of the new parameter `source_manifest=True`. # Test: Search for the usage of `source_manifest=True`. Expect: Only occurances of the new parameter. rg --type python $'source_manifest=True'Length of output: 70
Script:
#!/bin/bash # Description: Verify the usage of the new parameter `source_manifest=True`. # Test: Search for the usage of `source_manifest=True`. Expect: Only occurances of the new parameter. rg --type py 'source_manifest=True'Length of output: 580
14-14: LGTM! Verify the usage of the new import.The import statement has been updated correctly. Ensure that the usage of
get_sourcealigns with the new import.
tests/integration_tests/test_all_cache_types.py (4)
18-18: LGTM! Verify the usage of the new import.The import statement has been updated correctly. Ensure that the usage of
get_sourcealigns with the new import.<details> <summary>Verification successful</summary> --- <!-- [monologue] [task review_file_2/3 lines_18-18] The task is to verify the usage of the newly imported `get_source` function. [observations] The `rg` command output shows multiple instances of `get_source` being used across different files. Some of these instances import `get_source` from `airbyte.experimental`, while others use `ab.get_source`. [analysis] The goal is to ensure that the usage of `get_source` in `tests/integration_tests/test_all_cache_types.py` aligns with the new import from `airbyte.experimental`. The output indicates that the usage in this file is indeed consistent with the new import. [actions] Generate the final response indicating that the verification is successful. --> **Verified: The usage of `get_source` aligns with the new import.** The import statement has been updated correctly, and the usage of `get_source` in `tests/integration_tests/test_all_cache_types.py` is consistent with the new import from `airbyte.experimental`. - `tests/integration_tests/test_all_cache_types.py`: Lines 18, 20, 22, 24 </details> <details> <summary>Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Verify the usage of `get_source` aligns with the new import. # Test: Search for the usage of `get_source`. Expect: Only occurances of the new import. rg --type python $'get_source'Length of output: 60
Script:
#!/bin/bash # Description: Verify the usage of `get_source` aligns with the new import. # Test: Search for the usage of `get_source`. Expect: Only occurrences of the new import. rg --glob '*.py' 'get_source'Length of output: 8597
83-88: LGTM! Verify the usage of the new parameter.
The usage of get_source has been updated correctly. Ensure that the new parameter source_manifest=True is correctly used.
<details>
<summary>Verification successful</summary>
---
<!-- [monologue]
[task review_file_2/3 lines_83-88] Review the usage of `get_source` in `source_pokeapi`.
[observations] The `rg` command successfully found multiple occurrences of `source_manifest=True` in various files.
[analysis] The occurrences of `source_manifest=True` were found in different test files and example scripts, indicating that the new parameter is being used consistently across the codebase.
[actions] Conclude the verification process by confirming that the new parameter `source_manifest=True` is correctly used.
-->
**The new parameter `source_manifest=True` is correctly used across the codebase.**
The usage of `get_source` with the `source_manifest=True` parameter has been verified in multiple files:
- `tests/unit_tests/test_lowcode_connectors.py`
- `examples/run_pokeapi.py`
- `tests/integration_tests/test_all_cache_types.py`
- `examples/run_downloadable_yaml_source.py`
- `tests/integration_tests/test_lowcode_connectors.py`
</details>
<details>
<summary>Scripts executed</summary>
The following scripts were executed for the analysis:
Script:
```shell
#!/bin/bash
# Description: Verify the usage of the new parameter `source_manifest=True`.
# Test: Search for the usage of `source_manifest=True`. Expect: Only occurances of the new parameter.
rg --type python $'source_manifest=True'
Length of output: 70
Script:
#!/bin/bash
# Description: Verify the usage of the new parameter `source_manifest=True`.
# Test: Search for the usage of `source_manifest=True`. Expect: Only occurrences of the new parameter.
rg --type py $'source_manifest=True'
Length of output: 580
66-66: LGTM! Verify the usage of the new parameter.
The usage of get_source has been updated correctly. Ensure that the new parameter install_if_missing=False is correctly used.
<details>
<summary>Verification successful</summary>
---
<!-- [monologue]
[task review_file_2/3 lines_66-66] Verify the usage of the `install_if_missing=False` parameter in the `get_source` function.
[observations] The search results show multiple occurrences of the `install_if_missing=False` parameter across various test files, including `tests/integration_tests/test_all_cache_types.py`.
[analysis] The presence of the `install_if_missing=False` parameter in multiple test files indicates that the parameter is being used consistently. The specific file and line under review, `tests/integration_tests/test_all_cache_types.py` at line 66, also shows the correct usage of this parameter.
[actions] Conclude that the usage of the `install_if_missing=False` parameter is verified and correct.
-->
**LGTM! The `install_if_missing=False` parameter is used correctly.**
The parameter `install_if_missing=False` is consistently used across multiple test files, including the specified line in `tests/integration_tests/test_all_cache_types.py`.
- `tests/integration_tests/test_all_cache_types.py:66`
</details>
<details>
<summary>Scripts executed</summary>
The following scripts were executed for the analysis:
Script:
```shell
#!/bin/bash
# Description: Verify the usage of the new parameter `install_if_missing=False`.
# Test: Search for the usage of `install_if_missing=False`. Expect: Only occurances of the new parameter.
rg --type python $'install_if_missing=False'
Length of output: 74
Script:
#!/bin/bash
# Description: Verify the usage of the new parameter `install_if_missing=False`.
# Test: Search for the usage of `install_if_missing=False`. Expect: Only occurrences of the new parameter.
rg --type py 'install_if_missing=False'
Length of output: 1997
49-49: LGTM! Verify the usage of the new parameter.
The usage of get_source has been updated correctly. Ensure that the new parameter install_if_missing=False is correctly used.
<details>
<summary>Verification successful</summary>
---
<!-- [monologue]
[task review_file_2/3 lines_49-49] Verify the usage of `install_if_missing=False` in the codebase.
[observations] The search results show multiple instances of `install_if_missing=False` across various test files and one instance in a non-test file (`airbyte/validate.py`).
[analysis] The presence of `install_if_missing=False` in both unit and integration tests suggests that the new parameter is being used consistently across the codebase. The single instance in `airbyte/validate.py` also aligns with the expected usage. This confirms that the new parameter is correctly used.
[actions] Conclude the verification process and generate the final response indicating successful verification.
-->
**LGTM! Verify the usage of the new parameter.**
The usage of `get_source` has been updated correctly to include the `install_if_missing=False` parameter. The new parameter is consistently used across the codebase.
- `tests/unit_tests/test_anonymous_usage_stats.py`
- `tests/integration_tests/test_duckdb_cache.py`
- `tests/integration_tests/test_source_faker_integration.py`
- `tests/integration_tests/cloud/test_cloud_sync.py`
- `tests/integration_tests/test_all_cache_types.py`
- `tests/integration_tests/cloud/test_cloud_workspaces.py`
- `tests/integration_tests/test_source_test_fixture.py`
- `tests/integration_tests/cloud/test_cloud_sql_reads.py`
- `airbyte/validate.py`
</details>
<details>
<summary>Scripts executed</summary>
The following scripts were executed for the analysis:
Script:
```shell
#!/bin/bash
# Description: Verify the usage of the new parameter `install_if_missing=False`.
# Test: Search for the usage of `install_if_missing=False`. Expect: Only occurances of the new parameter.
rg --type python $'install_if_missing=False'
Length of output: 74
Script:
#!/bin/bash
# Description: Verify the usage of the new parameter `install_if_missing=False`.
# Test: Search for the usage of `install_if_missing=False`. Expect: Occurrences of the new parameter.
rg --type py 'install_if_missing=False'
Length of output: 1997
pyproject.toml (3)
61-61: LGTM! Verify the compatibility of the new version constraint.The version constraint for
airbyte-source-fakerhas been updated correctly. Ensure that the new version constraint">=6.2.0"is compatible with the project.<details> <summary>Verification successful</summary> **Verify the compatibility of the new version constraint for `airbyte-source-faker`.** The new version constraint `">=6.2.0"` for `airbyte-source-faker` is present in `pyproject.toml`. Ensure that this update does not introduce any breaking changes or incompatibilities with the rest of the project by checking the usage of `airbyte-source-faker` in the codebase. --- **Verify the consistency of the `poetry.lock` file with the new version constraint for `airbyte-source-faker`.** The new version constraint `">=6.2.0"` for `airbyte-source-faker` is present in `pyproject.toml`, and there are no direct usages of this package in the rest of the codebase. Ensure that the `poetry.lock` file is consistent with the new version constraint. --- <!-- [monologue] [task review_file_3/3 lines_61-61] Verify the consistency of the `poetry.lock` file with the new version constraint for `airbyte-source-faker`. [observations] The `poetry.lock` file shows that the version of `airbyte-source-faker` is `6.2.0`, which is consistent with the new version constraint `">=6.2.0"` specified in `pyproject.toml`. [analysis] The `poetry.lock` file is consistent with the new version constraint, and there are no direct usages of `airbyte-source-faker` in the codebase that could be affected by this version update. [actions] Conclude the verification process and categorize the final response. --> **The version constraint for `airbyte-source-faker` is correctly updated and consistent.** The `poetry.lock` file shows that the version of `airbyte-source-faker` is `6.2.0`, which aligns with the new version constraint `">=6.2.0"` specified in `pyproject.toml`. There are no direct usages of this package in the codebase that could be affected by this update. - `pyproject.toml`: `airbyte-source-faker = ">=6.2.0"` - `poetry.lock`: `version = "6.2.0"` </details> <details> <summary>Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Verify the compatibility of the new version constraint `">=6.2.0"` for `airbyte-source-faker`. # Test: Search for the usage of `airbyte-source-faker`. Expect: Only occurances of the new version constraint. rg --type toml $'airbyte-source-faker'Length of output: 85
Script:
#!/bin/bash # Search for the usage of `airbyte-source-faker` in the codebase to verify compatibility. rg 'airbyte-source-faker'Length of output: 115
Script:
#!/bin/bash # Check the version of `airbyte-source-faker` in the `poetry.lock` file to ensure consistency with the new version constraint. rg 'name = "airbyte-source-faker"' -A 5 poetry.lockLength of output: 234
18-18: LGTM! Verify the compatibility of the new version constraint.
The version constraint for airbyte-cdk has been updated correctly. Ensure that the new version constraint ">=2.0,<=3.0" is compatible with the project.
---
`32-32`: **LGTM! Verify the compatibility of the new version constraint.**
The version constraint for `pydantic` has been updated correctly. Ensure that the new version constraint `">=2.0,<=3.0"` is compatible with the project.
Verification successful
LGTM! Verify the compatibility of the new version constraint.
The version constraint for pydantic has been updated correctly. Ensure that the new version constraint ">=2.0,<=3.0" is compatible with the project by running the tests.
- Instances of
BaseModel,Field, andPrivateAttrfrompydanticwere found and appear to be used correctly. - Recommend running the project's tests to confirm compatibility.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the compatibility of the new version constraint `">=2.0,<=3.0"` for `pydantic`.
# Test: Search for the usage of `pydantic`. Expect: Only occurances of the new version constraint.
rg --type toml $'pydantic'
Length of output: 65
Script:
#!/bin/bash
# Search for all import statements of `pydantic` in the codebase to verify compatibility with the new version constraint.
rg --type py 'import pydantic' --context 5
Length of output: 44
Script:
#!/bin/bash
# Search for all from-import statements of `pydantic` in the codebase to verify compatibility with the new version constraint.
rg --type py 'from pydantic import' --context 5
Length of output: 7285
Script:
#!/bin/bash
# Search for the usage of `BaseModel` from `pydantic` in the codebase.
rg --type py 'BaseModel' --context 5
Length of output: 4298
Script:
#!/bin/bash
# Search for the usage of `Field` from `pydantic` in the codebase.
rg --type py 'Field' --context 5
Length of output: 12592
Script:
#!/bin/bash
# Search for the usage of `PrivateAttr` from `pydantic` in the codebase.
rg --type py 'PrivateAttr' --context 5
Length of output: 6419
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- airbyte/_util/document_rendering.py (2 hunks)
Files skipped from review due to trivial changes (1)
- airbyte/_util/document_rendering.py
|
CodeRabbit team member here - it looks like the formatting broke. :) |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- airbyte/caches/snowflake.py (1 hunks)
- airbyte/secrets/base.py (2 hunks)
Files skipped from review due to trivial changes (1)
- airbyte/caches/snowflake.py
Additional comments not posted (4)
airbyte/secrets/base.py (4)
9-11: Imports look good.The added imports for
TYPE_CHECKING,Any,CoreSchema, andcore_schemaare necessary for the new Pydantic compatibility methods.
77-89: Methodvalidatelooks good.The method correctly validates that the input value is a valid secret string and raises an appropriate exception if the value is not a string.
91-99: Method__get_pydantic_core_schema__looks good.The method correctly returns a core schema using the
validatemethod.
101-116: Method__get_pydantic_json_schema__looks good.The method correctly returns a JSON schema with
writeOnlyandformatproperties to prevent secrets from being exposed inadvertently.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- airbyte/_future_cdk/record_processor.py (1 hunks)
- airbyte/_future_cdk/state_providers.py (1 hunks)
- airbyte/_future_cdk/state_writers.py (1 hunks)
- airbyte/caches/_state_backend.py (2 hunks)
- airbyte/sources/base.py (4 hunks)
- airbyte/sources/declarative.py (4 hunks)
- airbyte/sources/registry.py (1 hunks)
- examples/run_faker.py (1 hunks)
Files skipped from review due to trivial changes (3)
- airbyte/_future_cdk/state_providers.py
- airbyte/sources/registry.py
- examples/run_faker.py
Additional comments not posted (10)
airbyte/_future_cdk/state_writers.py (1)
39-39: LGTM!The change to use
model_dump_jsoninstead ofjson()is appropriate for Pydantic 2.0.airbyte/sources/declarative.py (2)
25-34: LGTM!The function
_suppress_cdk_pydantic_deprecation_warningscorrectly suppresses the Pydantic deprecation warnings.
50-50: LGTM!The calls to
_suppress_cdk_pydantic_deprecation_warningsin the constructors ofDeclarativeExecutorandDeclarativeSourceare correctly placed.Also applies to: 110-110
airbyte/caches/_state_backend.py (2)
107-107: LGTM!The change to use
model_dump_jsoninstead ofjson()is appropriate for Pydantic 2.0.
173-173: LGTM!The change to use
model_validate_jsoninstead ofparse_rawis appropriate for Pydantic 2.0.airbyte/_future_cdk/record_processor.py (1)
136-136: LGTM!The change to use
model_validate_jsoninstead ofparse_rawis appropriate for Pydantic 2.0.airbyte/sources/base.py (4)
301-301: LGTM!The use of
model_dumpaligns with Pydantic 2.0 standards.
540-540: LGTM!The use of
model_dump_jsonaligns with Pydantic 2.0 standards.
582-582: LGTM!The use of
model_validate_jsonaligns with Pydantic 2.0 standards.
301-301: LGTM!The use of
model_dumpaligns with Pydantic 2.0 standards.
I saw that too! Good catch! Not a big problem, but I appreciate you are looking into it! 🙏 BTW, I'm learning to love CodeRabbit for our open source project, especially for PRs we review/approve ourselves (self-review) and for contributions from community members when I'm not deeply knowledgeable and/or when I just would appreciate another pair of eyes. Thanks for the great work! 🎉 |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/python_pytest.yml (1 hunks)
Files skipped from review due to trivial changes (1)
- .github/workflows/python_pytest.yml
Aaron ("AJ") Steers (aaronsteers)
linked an issue
2 participants
Summary by CodeRabbit
New Features
run_faker.pyfrom 50,000 to 200,000.Bug Fixes
Improvements
SecretString.Refactor