-
Notifications
You must be signed in to change notification settings - Fork 13
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
Update/pydantic v2 #411
Update/pydantic v2 #411
Conversation
…rmat in response body
WalkthroughThis pull request introduces a comprehensive update to the Cuenca Python library, focusing on modernizing the codebase and enhancing type safety. The changes span multiple areas, including GitHub Actions workflows, Python version updates, dependency management, and code structure. The primary modifications include upgrading Python versions from 3.8 to 3.9-3.13, updating dependencies, and refactoring type annotations across the library. The project now uses more modern Python type hints, replacing Workflow configurations have been updated to support newer Python versions, with GitHub Actions now testing across Python 3.9 through 3.13. The dependency requirements have been adjusted, removing older dependencies like The changes also involve updating method calls from 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
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: 7
🔭 Outside diff range comments (1)
cuenca/resources/user_events.py (1)
Update all schema() calls to model_json_schema() for Pydantic v2 compatibility
The codebase shows consistent migration to Pydantic v2 (using ConfigDict), but still uses the deprecated schema() method. Update all occurrences:
- cuenca/resources/user_events.py:
User.schema()
→User.model_json_schema()
- cuenca/resources/users.py:
TOSAgreement.schema()
→TOSAgreement.model_json_schema()
Address.schema()
→Address.model_json_schema()
KYCFile.schema()
→KYCFile.model_json_schema()
Beneficiary.schema()
→Beneficiary.model_json_schema()
🔗 Analysis chain
Line range hint
15-27
: Verify User.schema() compatibility with Pydantic v2.The schema example references
User.schema()
, but this method has changed in Pydantic v2.Consider updating to use
User.model_json_schema()
for Pydantic v2 compatibility:- 'new_model': User.schema().get('example'), + 'new_model': User.model_json_schema().get('example'),🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for User class definition and schema usage rg -l "class User" | xargs rg "schema\(\)"Length of output: 490
Script:
#!/bin/bash # Search for class definitions and their configurations echo "=== Searching for model definitions ===" ast-grep --pattern 'class $NAME($_) { $$$ model_config = ConfigDict( $$$ ) $$$ }' echo -e "\n=== Searching for model_json_schema usage ===" rg "model_json_schema\(\)"Length of output: 320
Script:
#!/bin/bash # Search for model class definitions and Pydantic imports echo "=== Checking Pydantic imports ===" rg "from pydantic" -A 2 echo -e "\n=== Looking for specific model classes ===" rg "class (User|TOSAgreement|Address|KYCFile|Beneficiary)" -A 5Length of output: 6543
🧹 Nitpick comments (10)
cuenca/resources/whatsapp_transfers.py (1)
Line range hint
25-31
: Consider using Pydantic's model_validator for destination property.Since we're migrating to Pydantic v2, consider replacing the property with a model validator for better type safety and validation. This would eliminate the need for manual casting.
- @property # type: ignore - def destination(self) -> Optional[Account]: - if self.destination_uri is None: - dest = None - else: - dest = cast(Account, retrieve_uri(self.destination_uri)) - return dest + @model_validator(mode='after') + def compute_destination(self) -> 'WhatsappTransfer': + if self.destination_uri is not None: + self._destination = retrieve_uri(self.destination_uri) + return self + + @property + def destination(self) -> Optional[Account]: + return getattr(self, '_destination', None)cuenca/resources/deposits.py (1)
16-16
: Consider using Pydantic validators for network-dependent tracking_key.The comment suggests that tracking_key is only relevant when network is SPEI. Consider using Pydantic's field validators to enforce this relationship:
from pydantic import Field, field_validator class Deposit(Transaction): tracking_key: Optional[str] = Field( default=None, description="Tracking key (clave rastreo) for SPEI network deposits" ) @field_validator('tracking_key') def validate_tracking_key(cls, v: Optional[str], info): if info.data.get('network') == DepositNetwork.SPEI and not v: raise ValueError('tracking_key is required for SPEI deposits') return vcuenca/resources/otps.py (1)
13-20
: LGTM! Consider adding migration guide reference.The migration from
Config
class toConfigDict
withjson_schema_extra
follows Pydantic v2 guidelines correctly.Consider adding a comment with a link to the Pydantic v2 migration guide for future reference:
# Migrated to Pydantic v2 ConfigDict # See: https://docs.pydantic.dev/latest/migration/#changes-to-configcuenca/resources/file_batches.py (1)
24-28
: Remove unnecessary type ignore comment.The type ignore comment seems unnecessary as
FileBatchUploadRequest
should handle the type conversion.- files=files, # type: ignore + files=files,Also, the migration from
dict()
tomodel_dump()
is correct for Pydantic v2.cuenca/resources/resources.py (1)
17-25
: Consider conditional threading based on test environment.While removing threading fixes VCR test issues, we could maintain the performance benefit in production by conditionally using threading:
def retrieve_uris(uris: list[str]) -> list[Retrievable]: + # Use threading in production, sequential in tests + if os.getenv('TESTING'): + return [retrieve_uri(uri) for uri in uris] + + with ThreadPoolExecutor() as executor: + return list(executor.map(retrieve_uri, uris)) - return [retrieve_uri(uri) for uri in uris]This maintains test stability while preserving concurrent execution benefits in production.
cuenca/resources/transfers.py (1)
1-1
: Overall: Well-executed Pydantic v2 migration.The changes consistently follow Pydantic v2 migration patterns across all files:
- Properly migrated from
dict()
tomodel_dump()
- Updated to
ConfigDict
for model configuration- Enhanced documentation using
Field
with descriptions- Modernized type hints for Python 3.9+
The migration maintains good consistency and improves code quality.
Consider adding migration notes to the documentation to help downstream projects update their code, especially regarding:
- The change from
dict()
tomodel_dump()
- The new date type handling in
Platform.create()
cuenca/resources/curp_validations.py (1)
97-97
: Remove type ignore comment.The type ignore comment for
country_of_birth
might not be necessary with Pydantic v2's improved type system.cuenca/resources/users.py (2)
40-70
: Consider using frozen=True for immutable fields.For fields that shouldn't change after creation (e.g.,
date_of_birth
,curp
), consider usingfrozen=True
in the Field definition to prevent accidental modifications.Example for immutable fields:
- curp: Optional[str] = None + curp: Optional[str] = Field(None, frozen=True) - date_of_birth: Optional[dt.date] = None + date_of_birth: Optional[dt.date] = Field(None, frozen=True)
Line range hint
77-101
: Update model_config to include all relevant Pydantic v2 settings.While the migration to
ConfigDict
is correct, consider adding other relevant Pydantic v2 settings:model_config = ConfigDict( + frozen=False, # Set to True if the model should be immutable + validate_assignment=True, # Validate fields on assignment + strict=True, # Enable strict type checking json_schema_extra={ 'example': { # ... existing example ... } } )tests/resources/cassettes/test_card_activation.yaml (1)
91-91
: Add newline at end of file.YAML files should end with a newline character.
version: 1 +
🧰 Tools
🪛 yamllint (1.35.1)
[error] 91-91: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (51)
.github/workflows/release.yml
(1 hunks).github/workflows/test.yml
(3 hunks)Makefile
(1 hunks)cuenca/resources/api_keys.py
(4 hunks)cuenca/resources/arpc.py
(3 hunks)cuenca/resources/balance_entries.py
(2 hunks)cuenca/resources/base.py
(7 hunks)cuenca/resources/card_activations.py
(3 hunks)cuenca/resources/card_transactions.py
(2 hunks)cuenca/resources/card_validations.py
(3 hunks)cuenca/resources/cards.py
(4 hunks)cuenca/resources/curp_validations.py
(4 hunks)cuenca/resources/deposits.py
(1 hunks)cuenca/resources/endpoints.py
(5 hunks)cuenca/resources/file_batches.py
(2 hunks)cuenca/resources/files.py
(1 hunks)cuenca/resources/identities.py
(1 hunks)cuenca/resources/kyc_validations.py
(3 hunks)cuenca/resources/kyc_verifications.py
(4 hunks)cuenca/resources/limited_wallets.py
(2 hunks)cuenca/resources/login_tokens.py
(1 hunks)cuenca/resources/otps.py
(2 hunks)cuenca/resources/platforms.py
(4 hunks)cuenca/resources/questionnaires.py
(3 hunks)cuenca/resources/resources.py
(2 hunks)cuenca/resources/savings.py
(3 hunks)cuenca/resources/service_providers.py
(2 hunks)cuenca/resources/sessions.py
(4 hunks)cuenca/resources/transfers.py
(3 hunks)cuenca/resources/user_credentials.py
(2 hunks)cuenca/resources/user_events.py
(3 hunks)cuenca/resources/user_lists_validation.py
(1 hunks)cuenca/resources/user_logins.py
(3 hunks)cuenca/resources/users.py
(9 hunks)cuenca/resources/verifications.py
(5 hunks)cuenca/resources/wallet_transactions.py
(1 hunks)cuenca/resources/webhooks.py
(1 hunks)cuenca/resources/whatsapp_transfers.py
(1 hunks)requirements-test.txt
(1 hunks)requirements.txt
(1 hunks)setup.py
(1 hunks)tests/conftest.py
(3 hunks)tests/resources/cassettes/test_card_activation.yaml
(1 hunks)tests/resources/cassettes/test_user_beneficiaries_update.yaml
(1 hunks)tests/resources/test_card_activations.py
(1 hunks)tests/resources/test_cards.py
(0 hunks)tests/resources/test_commissions.py
(2 hunks)tests/resources/test_endpoints.py
(1 hunks)tests/resources/test_otps.py
(1 hunks)tests/resources/test_sessions.py
(1 hunks)tests/resources/test_transfers.py
(1 hunks)
💤 Files with no reviewable changes (1)
- tests/resources/test_cards.py
✅ Files skipped from review due to trivial changes (1)
- tests/conftest.py
🧰 Additional context used
🪛 yamllint (1.35.1)
tests/resources/cassettes/test_card_activation.yaml
[error] 91-91: no new line character at the end of file
(new-line-at-end-of-file)
🪛 Ruff (0.8.2)
cuenca/resources/kyc_validations.py
35-35: Do not use mutable data structures for argument defaults
Replace with None
; initialize within function
(B006)
🔇 Additional comments (79)
cuenca/resources/login_tokens.py (2)
3-3
: LGTM! Correct import for Pydantic v2The import of
ConfigDict
aligns with Pydantic v2's recommended configuration approach.
12-14
: Configuration updated correctly for Pydantic v2The migration from
Config
class toConfigDict
follows Pydantic v2's recommended patterns.Let's verify if any additional Pydantic v2 configuration options should be considered:
✅ Verification successful
Configuration is consistent with codebase patterns
The implementation follows the project's standard configuration pattern and inherits additional settings from the base resource. No other configuration options need to be considered.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other Pydantic model configurations in the codebase to ensure consistency rg -l "ConfigDict" | while read -r file; do echo "=== $file ===" rg "ConfigDict\(" "$file" -A 2 doneLength of output: 3685
cuenca/resources/wallet_transactions.py (1)
38-38
: Migration to Pydantic v2 looks correct.The change from
dict()
tomodel_dump()
follows the correct migration pattern for Pydantic v2. This is a breaking change that's part of the broader migration effort.Let's verify if there are any direct usages of the old
dict()
method that might need similar updates:✅ Verification successful
Migration verified: No additional updates needed
The change from
dict()
tomodel_dump()
is correct and isolated. TheWalletTransactionRequest
type is imported from the already migratedcuenca_validations.types
package, and there are no other occurrences of.dict()
that need updating.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining uses of .dict() that might need migration rg --type python '\.dict\(\)' -C 2 # Search for any direct instantiations of WalletTransactionRequest to verify compatibility ast-grep --pattern 'WalletTransactionRequest($$$)'Length of output: 498
Script:
#!/bin/bash # Search for any remaining uses of .dict() that might need migration rg --type py '\.dict\(\)' -C 2 # Search for imports and usages of WalletTransactionRequest rg --type py 'WalletTransactionRequest' -C 2 # Find the definition of WalletTransactionRequest ast-grep --pattern 'class WalletTransactionRequest($$$)'Length of output: 842
cuenca/resources/whatsapp_transfers.py (2)
17-17
: LGTM! Pydantic v2 field initialization looks good.The changes align well with Pydantic v2 best practices by explicitly initializing optional fields with None defaults.
Also applies to: 20-22
24-24
: Consider removing the type: ignore comment.The
# type: ignore
comment on the property decorator might be unnecessary with newer Python versions and Pydantic v2. Let's verify if it can be removed.cuenca/resources/files.py (2)
47-47
: LGTM! Correct migration to Pydantic v2.The change from
dict()
tomodel_dump()
is the correct approach for Pydantic v2 migration. This aligns with the PR objectives and follows Pydantic's v2 best practices.Let's verify if all other Pydantic model instances have been migrated:
✅ Verification successful
Migration to model_dump() is complete and consistent
The codebase shows complete adoption of
model_dump()
with no remaining.dict()
calls, maintaining consistency across all resource files.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining uses of .dict() that might need migration # Look for potential missed migrations to model_dump() # Search for remaining .dict() calls that might need migration rg '\.dict\(\)' # Search for already migrated calls to ensure consistency rg '\.model_dump\(\)'Length of output: 3346
43-43
: Investigate the need for type ignore.The
type: ignore
comment suggests there might be a type compatibility issue. Please clarify why type checking needs to be ignored for the extension parameter, which is defined asOptional[str]
in the method signature.Let's check if there are any type-related issues or discussions:
✅ Verification successful
Type ignore comment is correctly used for extension parameter
The
type: ignore
comment is justified as it handles a known pattern where the method accepts an optional parameter (Optional[str]
) but passes it to a model that handles the validation internally. This is common in Pydantic usage where the model's validation layer manages the type conversion and validation rules.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for type-related issues with the extension parameter # Look for type hints, validation rules, or related discussions # Check for type hints and validation rules for extension parameter rg -A 5 "extension.*:.*str" # Look for FileUploadRequest definition to understand type constraints ast-grep --pattern 'class FileUploadRequest'Length of output: 649
Script:
#!/bin/bash # Get more context around the type ignore comment rg -B 5 -A 5 "type: ignore.*extension" # Find FileUploadRequest class definition with a broader pattern ast-grep --pattern 'class $_UploadRequest' # Look for where extension is used as a parameter rg -B 2 -A 2 "\bextension\s*=\s*"Length of output: 745
Script:
#!/bin/bash # Find the complete upload method implementation ast-grep --pattern 'upload($$$) { $$$ }' # Find classes with extension field ast-grep --pattern 'class $_ { $$$ extension: str $$$ }'Length of output: 109
Script:
#!/bin/bash # Get the complete content of the file cat cuenca/resources/files.py # Try a more flexible pattern for classes with extension ast-grep --pattern 'class $_ { extension }' # Look for upload method with different syntax ast-grep --pattern '@classmethod def upload'Length of output: 1935
cuenca/resources/service_providers.py (1)
13-13
: LGTM! Good use of PEP 585 built-in generics.The change from
typing.List
to built-inlist
type annotation is a good modernization that aligns with dropping Python 3.8 support, as built-in generics were introduced in Python 3.9.cuenca/resources/webhooks.py (1)
12-13
: LGTM! Good use of Pydantic v2 Field descriptions.The changes improve the code in several ways:
- Uses Pydantic's Field for proper metadata documentation
- Adopts PEP 585 built-in generics (dict instead of Dict)
- Adds clear field descriptions that enhance API documentation
tests/resources/test_otps.py (1)
28-28
: LGTM! Good use of isinstance() for type checking.The change from
type()
toisinstance()
is an improvement as it:
- Handles inheritance correctly
- Follows Python's EAFP (Easier to Ask for Forgiveness than Permission) principle
- Is the recommended way to perform runtime type checking
cuenca/resources/file_batches.py (1)
12-13
: LGTM! Type hints updated for Python 3.9+.The migration from
List[]
tolist[]
is correct for Python 3.9+.cuenca/resources/resources.py (1)
6-6
: LGTM! Type hint updated for Python 3.9+.The migration from
Dict[]
todict[]
is correct for Python 3.9+.tests/resources/test_commissions.py (1)
20-20
: Good improvement in type checking!The change from
type()
toisinstance()
is a better practice as it:
- Supports inheritance and polymorphism
- Follows Python's EAFP (Easier to Ask for Forgiveness than Permission) principle
- Aligns with type checking best practices
Also applies to: 31-31
cuenca/resources/limited_wallets.py (2)
40-40
: Pydantic v2 Migration: Updated to model_dump()Good update from
dict()
tomodel_dump()
for Pydantic v2 compatibility.
18-18
:⚠️ Potential issueBreaking Change:
allowed_curp
is now requiredThe
allowed_curp
parameter in thecreate
method is no longer optional. This is a breaking change that needs to be documented and communicated to users.Let's verify the impact:
Also applies to: 24-26
cuenca/resources/questionnaires.py (2)
5-5
: Correct Pydantic v2 configuration updatesGood migration to Pydantic v2:
- Replaced
Config
class withConfigDict
- Updated
schema_extra
tojson_schema_extra
Also applies to: 19-27
43-43
: Pydantic v2 Migration: Updated to model_dump()Good update from
dict()
tomodel_dump()
for Pydantic v2 compatibility.cuenca/resources/balance_entries.py (1)
1-1
: Improved type safety with UnionGood improvement in type hints:
- Changed from
TypeVar
toUnion
for more explicit type checking- This change will help static type checkers better understand the code
Also applies to: 11-11
tests/resources/test_card_activations.py (1)
11-21
: Verify test data updates.The test data has been updated with new values. Please ensure these are valid test credentials that won't interfere with production systems.
✅ Verification successful
Test data follows established patterns ✓
The updated test values follow the established conventions in the test suite:
- Card number uses the standard test BIN prefix (544875)
- User ID follows the consistent format (US + 20 chars) used across test files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the test card number pattern appears elsewhere in the codebase rg --type py "5448750001621241" --glob '!tests/resources/test_card_activations.py' # Check if the user ID pattern appears elsewhere rg --type py "US1w9BJ0DZ9kSdac39ur14Nf" --glob '!tests/resources/test_card_activations.py'Length of output: 173
Script:
#!/bin/bash # Check for similar card number patterns in tests rg --type py "5448[0-9]+" tests/ # Check for similar user ID patterns rg --type py "US[a-zA-Z0-9]{20}" tests/ # Look for test data configuration files fd -e py "test.*constants|constants.*test|test.*config|config.*test|test.*data|data.*test" tests/Length of output: 1178
cuenca/resources/card_transactions.py (2)
22-22
: Type annotations updated for Python 3.9+ compatibility.The changes from
List
tolist
align with Python 3.9+ type annotation syntax. The addition of default valueNone
toerror_type
follows Pydantic v2 best practices for optional fields.Also applies to: 27-27
30-34
: Return type annotation updated for consistency.The return type annotation change from
List
tolist
maintains consistency with Python 3.9+ typing syntax.cuenca/resources/identities.py (1)
26-39
: Optional fields updated with default values for Pydantic v2 compatibility.All Optional fields have been consistently updated with default
None
values, following Pydantic v2 best practices. This change improves model initialization and validation behavior.cuenca/resources/user_credentials.py (1)
28-28
: Updated to use Pydantic v2 model serialization API.The changes from
dict()
tomodel_dump()
align with Pydantic v2's API changes. Thedict()
method is deprecated in v2 in favor ofmodel_dump()
.Note: Please ensure all other occurrences of
dict()
in the codebase are also migrated tomodel_dump()
.Also applies to: 43-43
✅ Verification successful
All occurrences of
dict()
have been migrated tomodel_dump()
The codebase has been fully migrated to use Pydantic v2's
model_dump()
API. No remaining instances of the deprecateddict()
method were found in production code.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for remaining dict() calls that might need migration rg --type py "\.dict\(\)" --glob '!tests/**'Length of output: 46
Script:
#!/bin/bash # Search for Pydantic model usage and serialization patterns # Look for both old and new API usage rg --type py "from pydantic" -A 2 ast-grep --pattern 'BaseModel' rg --type py "model_dump" --glob '!tests/**'Length of output: 8065
setup.py (1)
24-24
: LGTM: Python version updates align with PR objectives.The changes correctly implement the dropping of Python 3.8 support by:
- Setting minimum Python version to 3.9
- Updating classifiers to reflect supported versions (3.9-3.13)
Also applies to: 31-35
tests/resources/test_sessions.py (1)
10-10
: LGTM: Type hint updated to use built-indict
.The change from
Dict
todict
aligns with modern Python type hinting practices.cuenca/resources/user_lists_validation.py (2)
39-39
: LGTM: Updated to use Pydantic v2's model_dump().Correctly migrated from deprecated
dict()
tomodel_dump()
as per Pydantic v2.
37-37
: Investigate the need for type ignore.The
# type: ignore
comment suggests a type compatibility issue that should be investigated rather than suppressed.Let's check the type definition:
cuenca/resources/kyc_validations.py (3)
1-1
: LGTM: Successfully migrated to Pydantic v2 configuration.Changes correctly implement Pydantic v2 patterns:
- Using ConfigDict instead of Config class
- Updated to json_schema_extra
- Removed typing.List import in favor of built-in list
Also applies to: 4-4, 17-28
13-15
: LGTM: Added default None to Optional fields.Good practice to explicitly set None as default for Optional fields.
43-43
: LGTM: Updated to use Pydantic v2's model_dump().Correctly migrated from deprecated
dict()
tomodel_dump()
as per Pydantic v2.cuenca/resources/card_activations.py (3)
5-5
: Great use of PaymentCardNumber type!Using
PaymentCardNumber
frompydantic_extra_types.payment
provides better validation for card numbers compared to the basicstr
type.Also applies to: 25-25
19-19
: LGTM: Optional field initializationAdding default
None
to Optional fields is the recommended approach in Pydantic v2.
46-46
: Correct usage of model_dump()Good replacement of
dict()
withmodel_dump()
as per Pydantic v2 guidelines.cuenca/resources/user_logins.py (3)
17-25
: Good migration to model_configProper migration from
Config
class tomodel_config
usingConfigDict
. The JSON schema example is well-structured.
14-14
: LGTM: Optional field initializationGood practice to initialize Optional fields with None in Pydantic v2.
36-36
: Correct usage of model_dump()Appropriate replacement of
dict()
withmodel_dump()
for Pydantic v2.cuenca/resources/savings.py (2)
20-21
: LGTM: Optional field initializationGood practice to initialize Optional fields with None in Pydantic v2.
37-37
: Consistent usage of model_dump()Good consistency in replacing
dict()
withmodel_dump()
across both create and update methods.Also applies to: 54-54
cuenca/resources/kyc_verifications.py (4)
21-22
: LGTM: Optional field initializationGood practice to initialize Optional fields with None in Pydantic v2.
Line range hint
27-40
: Good migration to model_configProper migration from
Config
class tomodel_config
usingConfigDict
. The JSON schema example is well-structured.
53-53
: Correct usage of model_dump()Good replacement of
dict()
withmodel_dump()
as per Pydantic v2 guidelines.
50-50
: Verify breaking change in update methodThe
curp
parameter has been changed fromOptional[CurpField]
to requiredCurpField
. This is a breaking change that might affect existing code.Let's check for existing usages:
✅ Verification successful
No breaking change detected in practice
The change from
Optional[CurpField]
to requiredCurpField
aligns with the existing usage pattern. The only usage ofKYCVerification.update()
in the codebase (in tests/resources/test_kyc_verifications.py) already passescurp
as a required parameter.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for calls to KYCVerification.update rg -l "KYCVerification.update" | xargs rg "KYCVerification\.update\("Length of output: 134
Script:
#!/bin/bash # Search for calls with more context rg -B 3 -A 3 "KYCVerification\.update\(" # Find the class definition and update method ast-grep --pattern 'class KYCVerification { $$$ update($$$) { $$$ } $$$ }' # Look for test files rg -l "test.*KYCVerification"Length of output: 726
Script:
#!/bin/bash # Try different pattern for class definition ast-grep --pattern 'class KYCVerification' # Search for any assignments or imports rg -B 2 -A 2 "from.*KYCVerification|import.*KYCVerification" # Search for any variable assignments rg "kyc.*=.*KYCVerification" -B 2 -A 2Length of output: 1928
cuenca/resources/sessions.py (3)
Line range hint
23-36
: LGTM! Correct migration to Pydantic v2 model configuration.The migration from
Config
class tomodel_config
usingConfigDict
follows Pydantic v2 best practices.
54-54
: LGTM! Correct usage of model_dump().The change from
dict()
tomodel_dump()
aligns with Pydantic v2's API.
51-52
: Investigate type ignore comments for URL fields.The type ignore comments on
success_url
andfailure_url
suggest potential type compatibility issues. Consider using proper type coercion or validation instead.Let's check if these type ignores are still necessary with Pydantic v2:
✅ Verification successful
Type ignores are correctly used for URL fields
The type ignores are necessary because the method accepts string URLs but the model uses Pydantic's AnyUrl type. Pydantic handles the string-to-AnyUrl conversion at runtime, making this a valid pattern similar to how headers are handled in the HTTP client.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar URL field handling patterns rg -l "AnyUrl.*type:\s*ignore" .Length of output: 34
Script:
#!/bin/bash # Check for URL-related imports and type annotations rg -A 2 "from pydantic.*Url" . # Look for Session class definition and fields ast-grep --pattern 'class Session: $$$ success_url: $_ = $_ $$$' # Search for type ignore patterns with URL rg -i "url.*=.*#.*type:\s*ignore" .Length of output: 1038
Script:
#!/bin/bash # Get Session class definition ast-grep --pattern 'class Session: $$$' # Get context where success_url and failure_url are used rg -B 3 -A 3 "success_url.*type:\s*ignore|failure_url.*type:\s*ignore" .Length of output: 7056
Script:
#!/bin/bash # Get SessionRequest definition ast-grep --pattern 'class SessionRequest: $$$' # Look for URL field type annotations rg -B 2 -A 2 "success_url.*:|failure_url.*:" .Length of output: 3354
cuenca/resources/verifications.py (2)
19-21
: LGTM! Good documentation improvement.Adding a description to the
recipient
field usingField
improves API documentation.
Line range hint
25-35
: LGTM! Correct schema configuration.The migration to
model_config
withConfigDict
and the schema example are well-structured.cuenca/resources/arpc.py (2)
33-33
: LGTM! Enhanced type safety for card numbers.The change from
str
toPaymentCardNumber
improves type safety and validation.
54-54
: Investigate type ignore on track_data_method.The type ignore comment suggests potential type compatibility issues with
track_data_method
.Let's check the type definition:
cuenca/resources/card_validations.py (2)
34-34
: LGTM! Improved type safety and consistency.Good improvements:
- Changed
number
fromstr
toPaymentCardNumber
for better type safety- Changed
pin_attempts_exceeded
fromstr
tobool
for correct type representationAlso applies to: 41-41
22-26
: LGTM! Consistent Optional boolean fields.All Optional boolean validation fields now consistently default to None.
cuenca/resources/api_keys.py (3)
16-17
: LGTM! Default values improve code clarity.The addition of default values for optional fields
deactivated_at
anduser_id
makes the initialization behavior explicit.
Line range hint
18-29
: LGTM! ConfigDict usage aligns with Pydantic v2.The migration from
Config
class toConfigDict
follows Pydantic v2's recommended approach for model configuration.
78-78
: LGTM! Updated to Pydantic v2's model_dump().Correctly migrated from deprecated
dict()
tomodel_dump()
for model serialization.cuenca/resources/platforms.py (3)
15-34
: LGTM! Enhanced field documentation with Pydantic Field.Good use of
Field
with descriptions improves API documentation and code clarity.
76-76
: LGTM! Updated to Pydantic v2's model_dump().Correctly migrated from deprecated
dict()
tomodel_dump()
.
57-57
: Verify establishment_date type change impact.The parameter type changed from
Optional[str]
toOptional[dt.date]
. While this is more type-safe, ensure all clients are updated to handle date objects.cuenca/resources/endpoints.py (2)
17-29
: LGTM! Well-documented fields with clear descriptions.Good use of
Field
with detailed descriptions improves API documentation.
55-55
: Verify Python version compatibility for list type hints.The change from
List
tolist
type hints requires Python 3.9+. Since Python 3.8 support is being dropped, this change is appropriate.Also applies to: 76-76
✅ Verification successful
Type hint changes are compatible with project requirements
The codebase explicitly requires Python >=3.9 in setup.py, and no usage of capital-case
List
type hints remains. The change to lowercaselist
type hints is appropriate.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any remaining usage of typing.List rg "from typing import.*List" rg "List\["Length of output: 45
Script:
#!/bin/bash # Check Python version requirements and type hint usage fd "setup.py|pyproject.toml|requirements.txt" --type f --exec cat {} rg "from typing import" -A 3 rg "Dict\[|Set\[|Tuple\[|Union\[|Optional\[" --type pyLength of output: 26389
cuenca/resources/transfers.py (3)
28-28
: LGTM! Explicit default value improves clarity.Good addition of default value for optional tracking_key with clear comment about its usage.
75-75
: LGTM! Updated type hint to use built-in list.Appropriate use of
list
type hint, consistent with dropping Python 3.8 support.
72-72
: LGTM! Consistent use of model_dump().Correctly migrated from deprecated
dict()
tomodel_dump()
in both create and create_many methods.Also applies to: 79-79
cuenca/resources/cards.py (3)
12-12
: LGTM! Enhanced type safety for card numbers.The addition of
PaymentCardNumber
from pydantic_extra_types improves validation for card numbers.
25-26
: LGTM! Improved attribute definitions.The updates to attribute types and default values align with Pydantic v2 best practices:
- Optional fields explicitly initialized with
None
- Strong typing with
PaymentCardNumber
for better validationAlso applies to: 30-30
85-85
: LGTM! Updated to Pydantic v2 serialization.Replaced
dict()
withmodel_dump()
as per Pydantic v2 guidelines.Also applies to: 110-110
cuenca/resources/curp_validations.py (2)
11-11
: LGTM! Enhanced schema documentation.Excellent use of Pydantic's
Field
with descriptions for all attributes. This improves:
- API documentation
- Schema validation
- Code maintainability
Also applies to: 21-54
Line range hint
55-75
: LGTM! Updated model configuration.Properly migrated to Pydantic v2's
ConfigDict
for model configuration while maintaining the example schema.tests/resources/test_transfers.py (1)
132-132
: LGTM! Updated error message assertion.Correctly updated the test to match Pydantic v2's new error message format for validation errors.
cuenca/resources/base.py (3)
15-15
: LGTM! Updated model configuration.Properly migrated to Pydantic v2's:
- New imports
ConfigDict
for model configurationAlso applies to: 28-30
33-33
: LGTM! Consistent serialization updates.Systematically replaced all instances of
dict()
withmodel_dump()
across the codebase, following Pydantic v2 guidelines.Also applies to: 161-161, 178-178, 195-195, 207-207
82-82
: LGTM! Explicit optional field initialization.Added default value of
None
for optionaldeactivated_at
field, improving clarity and alignment with Pydantic v2 practices.cuenca/resources/users.py (2)
33-38
: LGTM! Field definitions with descriptions improve code documentation.The use of
Field
with descriptions forlevel
andrequired_level
follows Pydantic v2 best practices and enhances code documentation.
133-133
: Verify model_dump() usage across the codebase.The migration from
dict()
tomodel_dump()
is correct. However, ensure all other model serialization calls in the codebase are updated.Also applies to: 172-172
✅ Verification successful
All model serialization calls use model_dump() consistently
The codebase has been fully migrated from
dict()
tomodel_dump()
. No remaining instances ofdict()
were found for Pydantic model serialization, andmodel_dump()
is consistently used across all resource files.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for remaining dict() calls that might need migration rg '\.dict\(' --type pyLength of output: 25
Script:
#!/bin/bash # Search for both dict() and model_dump() with context echo "=== Searching for .dict() calls ===" rg '\.dict\s*\(' -A 2 --type py echo -e "\n=== Searching for .model_dump() calls ===" rg '\.model_dump\s*\(' -A 2 --type py echo -e "\n=== Searching for Pydantic BaseModel usage ===" rg 'class.*\(.*BaseModel.*\):' -A 5 --type py echo -e "\n=== Searching for from pydantic import ===" rg '^from pydantic import' --type pyLength of output: 7480
requirements-test.txt (1)
1-12
: Verify compatibility with major version updates.Several dependencies have major version updates that might introduce breaking changes:
- flake8: 4.x → 7.x
- pytest: 6.x → 8.x
- vcrpy: 4.x → 7.x
Please verify test suite compatibility with these versions.
✅ Verification successful
Dependencies appear safe to update
The codebase uses standard patterns for all tools:
- pytest: Only basic fixtures and common decorators
- pytest-vcr: Simple @pytest.mark.vcr usage
- flake8: No direct API usage
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for deprecated features or breaking changes in test files # Look for potential vcrpy breaking changes rg "vcr\." --type py # Look for potential pytest breaking changes rg "@pytest\." --type pyLength of output: 6962
.github/workflows/release.yml (1)
20-20
: LGTM! Using tagged version for PyPI publish action.Using
release/v1
instead ofmaster
ensures stability and reproducibility..github/workflows/test.yml (1)
10-14
: LGTM! Action versions are up to date.The updates to GitHub Actions versions are appropriate:
- Using
actions/setup-python@v5
- Using
codecov/codecov-action@v5
Also applies to: 27-29, 39-42, 48-54
tests/resources/test_endpoints.py (1)
46-46
: Verify URL handling after Pydantic v2 migration.The change from direct string comparison to using
unicode_string()
suggests thatendpoint.url
is now a Pydantic URL type. This is consistent with Pydantic v2 migration. However, verify that:
- All URL comparisons in tests are updated similarly
- The trailing slash in the expected URL is intentional
✅ Verification successful
URL handling changes are correct ✅
The codebase scan shows this is the only test comparing URL string values, while other URL-related tests only verify existence. The
unicode_string()
usage is appropriate here as bothfiles.py
andendpoints.py
define URLs using Pydantic'sHttpUrl
type.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find other URL string comparisons that might need updates rg -l "assert.*\.url\s*==" tests/Length of output: 35
Script:
#!/bin/bash # Search for URL-related tests and assertions with broader patterns echo "=== URL-related assertions ===" rg -i "assert.*url" tests/ echo -e "\n=== URL field definitions ===" ast-grep --pattern 'url: $$$' echo -e "\n=== unicode_string usage ===" rg "unicode_string" .Length of output: 901
tests/resources/cassettes/test_user_beneficiaries_update.yaml (1)
32-32
: Test fixture updates look good.The changes to phone number format and addition of timestamps are consistent with the API response structure.
tests/resources/cassettes/test_card_activation.yaml (1)
3-17
: Authentication header change looks good.The update from
X-Cuenca-LoginId
toX-Cuenca-LoginToken
appears to be an intentional API change. Test data updates are consistent with the new format.Also applies to: 22-22
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #411 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 49 49
Lines 1101 1087 -14
=========================================
- Hits 1101 1087 -14
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
…urp type instead of CurpField
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: 1
🧹 Nitpick comments (1)
cuenca/resources/limited_wallets.py (1)
Line range hint
1-41
: Consider adding migration notes for breaking changes.Since this PR introduces breaking changes as part of the Pydantic v2 migration, consider:
- Adding migration notes to the documentation
- Bumping the major version number
- Including examples of how to update client code
Would you like me to help draft migration notes or examples for the documentation?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
cuenca/resources/cards.py
(3 hunks)cuenca/resources/curp_validations.py
(5 hunks)cuenca/resources/identities.py
(2 hunks)cuenca/resources/kyc_verifications.py
(4 hunks)cuenca/resources/limited_wallets.py
(3 hunks)cuenca/resources/user_lists_validation.py
(4 hunks)cuenca/resources/users.py
(9 hunks)requirements.txt
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- requirements.txt
- cuenca/resources/cards.py
- cuenca/resources/curp_validations.py
- cuenca/resources/user_lists_validation.py
- cuenca/resources/kyc_verifications.py
- cuenca/resources/users.py
- cuenca/resources/identities.py
🔇 Additional comments (3)
cuenca/resources/limited_wallets.py (3)
6-6
: LGTM! Import changes align with Pydantic v2 migration.The change from
CurpField
toCurp
is consistent with the migration to Pydantic v2.
24-25
: LGTM! Method signature changes are consistent with attribute updates.The parameter changes align with the attribute modifications and are well-documented in the docstring.
40-40
: LGTM! Updated to use Pydantic v2's model_dump().The change from
dict()
tomodel_dump()
follows Pydantic v2's recommended serialization approach.
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: 1
♻️ Duplicate comments (1)
cuenca/resources/limited_wallets.py (1)
18-19
:⚠️ Potential issueBreaking change detected in attribute declarations.
Making
allowed_curp
non-optional is a breaking change that could impact existing code. Consider keeping it optional or provide a migration guide for users.Apply this diff to maintain backward compatibility:
- allowed_curp: Curp + allowed_curp: Optional[Curp] = None
🧹 Nitpick comments (1)
cuenca/resources/platforms.py (1)
Line range hint
35-50
: Update example date formats in schema.The example data uses full datetime strings (
2021-08-24T14:15:22Z
) for theestablishment_date
field, but the model expects a date. Consider updating the example to use the correct format:'id': 'PT0123456789', 'name': 'Arteria', 'created_at': '2021-08-24T14:15:22Z', 'rfc': 'ART123456FFF', - 'establishment_date': '2021-08-24T14:15:22Z', + 'establishment_date': '2021-08-24', 'country': 'MX',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cuenca/resources/card_validations.py
(2 hunks)cuenca/resources/kyc_verifications.py
(4 hunks)cuenca/resources/limited_wallets.py
(3 hunks)cuenca/resources/platforms.py
(4 hunks)cuenca/resources/users.py
(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- cuenca/resources/kyc_verifications.py
- cuenca/resources/card_validations.py
- cuenca/resources/users.py
🔇 Additional comments (7)
cuenca/resources/limited_wallets.py (3)
6-6
: LGTM: Import updated for Pydantic v2 compatibility.The import statement has been correctly updated to use the new type definitions.
40-40
: LGTM: Updated to use Pydantic v2's model_dump().The change from
dict()
tomodel_dump()
aligns with Pydantic v2's best practices for model serialization.
24-24
:⚠️ Potential issueFix inconsistency between class attribute and method parameter.
The
allowed_curp
parameter increate()
is optional (Optional[Curp] = None
) while the class attribute requires it (Curp
). This inconsistency could lead to runtime errors when the method is called without the parameter.Either:
- Make the parameter required to match the class attribute:
- allowed_curp: Optional[Curp] = None, + allowed_curp: Curp,
- Or make the class attribute optional to match the method parameter (recommended to maintain backward compatibility).
Likely invalid or redundant comment.
cuenca/resources/platforms.py (4)
5-5
: LGTM! Proper Pydantic v2 imports.The imports correctly use Pydantic v2's new structure with
ConfigDict
for model configuration andField
for enhanced field definitions.
15-34
: LGTM! Well-documented field definitions using Pydantic v2 Field.The field definitions follow Pydantic v2 best practices with clear descriptions that will be reflected in the generated schema.
76-76
: LGTM! Proper use of Pydantic v2 model_dump().Correctly uses
model_dump()
instead of the deprecateddict()
method, following Pydantic v2 guidelines.
Line range hint
1-76
: Verify Python version requirements.Since this PR drops Python 3.8 support, ensure that:
- The package's
setup.py
orpyproject.toml
is updated to reflect the new minimum Python version- CI/CD workflows are updated to remove Python 3.8 testing
✅ Verification successful
Python version requirements are correctly updated
The package's
setup.py
has been properly updated to drop Python 3.8 support:
- Minimum Python version is set to
>=3.9
- Supported versions are explicitly listed as 3.9 through 3.13
Note: CI workflow configurations could not be verified as no workflow files were found in the repository. Consider adding appropriate CI configurations if they don't exist yet.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check Python version specifications in project configuration echo "Checking Python version requirements..." fd -e toml -e cfg -e ini -x cat {} \; | grep -i "python" fd "setup.py" -x cat {} \; | grep -i "python" # Check CI workflow configurations echo "Checking CI workflow Python versions..." fd -e yml -e yaml -p ".github/workflows" -x cat {} \; | grep -A 5 "python-version"Length of output: 700
Script:
#!/bin/bash # Check CI workflow files with broader pattern echo "CI Workflow Contents:" fd -e yml -e yaml -p ".github/workflows" -x cat {}Length of output: 105
Description:
This PR migrates the project to Pydantic v2 and drops support for Python 3.8
Key Changes:
Breaking Changes:
Summary by CodeRabbit
Based on the comprehensive summary of changes, here are the high-level release notes:
Python Version Upgrade
Dependency Updates
requests
librarycuenca-validations
pydantic-extra-types
dataclasses
dependencyType Annotations and Modernization
List
andDict
with built-inlist
anddict
Config
class tomodel_config
withConfigDict
Data Handling
.dict()
method calls with.model_dump()
GitHub Actions and CI/CD
Testing
These release notes provide a high-level overview of the significant changes in the library, focusing on improvements in Python version support, dependency management, type handling, and overall code modernization.