Update/pydantic v2 #410
Update/pydantic v2 #410
Conversation
WalkthroughThis pull request introduces a comprehensive update to the Cuenca Python library, focusing on modernizing the codebase and enhancing compatibility with newer Python versions. The changes span multiple areas, including GitHub Actions workflows, project configuration, and resource classes. The primary modifications involve updating Python version support from 3.8 to a range of versions including 3.9 through 3.13, transitioning to Pydantic v2 configuration patterns, and refining type handling across various resource classes. The update includes changes to dependency versions, workflow configurations, and numerous resource class implementations, emphasizing improved type safety, more explicit attribute initialization, and alignment with modern Python development practices. Finishing Touches
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #410 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 49 49
Lines 1111 1104 -7
=========================================
- Hits 1111 1104 -7
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
…es extraction in batch_transfers.py
…for compatibility with Pydantic v2
…hreadPoolExecutor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (4)
cuenca/resources/limited_wallets.py (1)
Line range hint
41-41: Update to Pydantic v2 model_dump()Replace
request.dict()withrequest.model_dump()to align with Pydantic v2 API changes.- return cast('LimitedWallet', cls._create(**request.dict())) + return cast('LimitedWallet', cls._create(**request.model_dump()))cuenca/resources/kyc_validations.py (1)
Line range hint
41-42: Update dict() to model_dump() for Pydantic v2 compatibility.The
createmethod still uses the Pydantic v1dict()method. This should be updated tomodel_dump()for v2 compatibility.- 'KYCValidation', cls._create(**req.dict(), session=session) + 'KYCValidation', cls._create(**req.model_dump(), session=session)cuenca/resources/savings.py (1)
Line range hint
35-35: Update dict() calls to model_dump() for Pydantic v2 compatibility.The
dict()method is deprecated in Pydantic v2.Apply these changes:
- return cast('Saving', cls._create(**request.dict())) + return cast('Saving', cls._create(**request.model_dump())) - return cast('Saving', cls._update(id=saving_id, **request.dict())) + return cast('Saving', cls._update(id=saving_id, **request.model_dump()))Also applies to: 49-49
cuenca/resources/files.py (1)
Line range hint
54-54: Migrate to model_dump().The
dict()method is deprecated in Pydantic v2. Update to usemodel_dump()instead.Apply this diff:
- **req.dict(), + **req.model_dump(),
🧹 Nitpick comments (8)
cuenca/resources/webhooks.py (1)
12-17: LGTM! Consider using computed default factory for payload.The migration to Pydantic v2 style field declarations looks good. For better type safety and maintainability, consider using a computed default factory for the
payloadfield to ensure a new dict instance for each model:- payload: Dict[str, Any] = Field( - ..., description='object sent by the webhook' - ) + payload: Dict[str, Any] = Field( + default_factory=dict, description='object sent by the webhook' + )cuenca/resources/deposits.py (1)
16-16: LGTM! Consider using Field for better documentation.The explicit default of
Noneimproves clarity. Consider using Pydantic'sFieldto move the documentation from a comment to the field's metadata:- tracking_key: Optional[str] = None # clave rastreo if network is SPEI + tracking_key: Optional[str] = Field( + default=None, + description='clave rastreo (tracking key) when network is SPEI' + )cuenca/resources/user_events.py (1)
3-3: LGTM! Consider adding migration notes.The changes correctly implement Pydantic v2 patterns by:
- Using
ConfigDictinstead of innerConfigclass- Using
json_schema_extrainstead ofschema_extraConsider adding a comment or documentation note about the Pydantic v2 migration to help other developers understand the new patterns.
Also applies to: 14-26
cuenca/resources/file_batches.py (1)
3-7: LGTM! Consider improving type annotations.The changes correctly implement Pydantic v2 patterns:
- Using
model_dump()instead ofdict()- Enhanced type safety with explicit casting
Consider updating the method signature to use
List[FileRequest]instead ofList[Dict]to make the type requirements more explicit:@classmethod def create( cls, - files: List[Dict], + files: List[FileRequest], user_id: str, *, session: Session = global_session, ) -> 'FileBatch': req = FileBatchUploadRequest( - files=cast(List[FileRequest], files), + files=files, user_id=user_id )Also applies to: 28-33
cuenca/resources/identities.py (1)
26-39: LGTM: Consistent default values for Optional fieldsThe addition of default
Nonevalues for Optional fields aligns with Pydantic v2 best practices and improves code consistency.Add Pydantic v2 model configuration
Consider adding the new Pydantic v2 model configuration:
class Identity(Retrievable, Queryable): _resource: ClassVar = 'identities' _query_params: ClassVar = IdentityQuery + model_config = ConfigDict( + populate_by_name=True, + validate_assignment=True + )cuenca/resources/card_transactions.py (1)
30-30: LGTM: Default value for Optional fieldAdding default
Noneforerror_typealigns with Pydantic v2 best practices.Add Pydantic v2 model configuration
Consider adding the new Pydantic v2 model configuration:
class CardTransaction(Transaction): + model_config = ConfigDict( + populate_by_name=True, + validate_assignment=True + )cuenca/resources/transfers.py (1)
94-94: Consider using a more robust idempotency key formatThe current format using string concatenation with date and amount might not guarantee uniqueness in high-frequency scenarios.
Consider using a more robust format:
- return f'{dt.datetime.utcnow().date()}: {account_number}: {amount}' + return f'{dt.datetime.utcnow().isoformat()}-{account_number}-{amount}'.github/workflows/test.yml (1)
Line range hint
47-51: Update Codecov token handling.Consider using GITHUB_TOKEN instead of a separate CODECOV_TOKEN for better security. Recent versions of Codecov support this approach.
file: ./coverage.xml flags: unittests name: codecov-umbrella - token: ${{ secrets.CODECOV_TOKEN }} + token: ${{ secrets.GITHUB_TOKEN }} fail_ci_if_error: true
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (41)
.github/workflows/release.yml(1 hunks).github/workflows/test.yml(3 hunks)Makefile(1 hunks)cuenca/resources/api_keys.py(3 hunks)cuenca/resources/arpc.py(3 hunks)cuenca/resources/balance_entries.py(2 hunks)cuenca/resources/base.py(4 hunks)cuenca/resources/card_activations.py(2 hunks)cuenca/resources/card_transactions.py(2 hunks)cuenca/resources/card_validations.py(3 hunks)cuenca/resources/cards.py(1 hunks)cuenca/resources/curp_validations.py(4 hunks)cuenca/resources/deposits.py(1 hunks)cuenca/resources/endpoints.py(2 hunks)cuenca/resources/file_batches.py(2 hunks)cuenca/resources/files.py(2 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(1 hunks)cuenca/resources/login_tokens.py(1 hunks)cuenca/resources/otps.py(1 hunks)cuenca/resources/platforms.py(4 hunks)cuenca/resources/questionnaires.py(3 hunks)cuenca/resources/resources.py(1 hunks)cuenca/resources/savings.py(1 hunks)cuenca/resources/sessions.py(3 hunks)cuenca/resources/transfers.py(4 hunks)cuenca/resources/user_events.py(3 hunks)cuenca/resources/user_lists_validation.py(2 hunks)cuenca/resources/user_logins.py(2 hunks)cuenca/resources/users.py(7 hunks)cuenca/resources/verifications.py(4 hunks)cuenca/resources/webhooks.py(1 hunks)cuenca/resources/whatsapp_transfers.py(1 hunks)cuenca/version.py(1 hunks)requirements-test.txt(1 hunks)requirements.txt(1 hunks)setup.py(1 hunks)tests/resources/test_endpoints.py(1 hunks)tests/resources/test_transfers.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- cuenca/version.py
🧰 Additional context used
🪛 Ruff (0.8.2)
cuenca/resources/sessions.py
50-50: Do not perform function call cast in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
51-51: Do not perform function call cast in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🔇 Additional comments (57)
cuenca/resources/questionnaires.py (4)
5-5: LGTM! Correct import for Pydantic v2 configurationThe import of
ConfigDictis appropriate for the migration to Pydantic v2.
43-43: LGTM! Correct usage of model_dump() for Pydantic v2The change from
dict()tomodel_dump()aligns with Pydantic v2's API changes.
18-26: Verify schema example completenessWhile the migration to
model_configandjson_schema_extrais correct for Pydantic v2, the example schema is missing theform_idfield that's required in the create method.Add the missing field to the example:
model_config = ConfigDict( json_schema_extra={ 'example': { 'user_id': 'US234i23jh23h4h23', 'token': '3223j23ij23ij3', + 'form_id': 'FMewifjwiejf', - 'alert_id': 'ALewifjwiejf', # This seems to be an incorrect field } } )✅ Verification successful
Schema example needs update as suggested
The review comment is correct. The
form_idfield is required in the class and create method, whilealert_idis not used anywhere in the codebase. The suggested changes to addform_idand removealert_idfrom the example schema are appropriate.🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Find the complete class definition and its fields ast-grep --pattern 'class $_ { $$$ model_config = ConfigDict( $$$ ) $$$ }' # Search for create method and its parameters ast-grep --pattern 'async def create( $$$ )' # Look for usage patterns of form_id and alert_id rg "form_id|alert_id" -A 2 -B 2Length of output: 38822
Line range hint
10-10: Verify parent classes compatibility with Pydantic v2Since this class inherits from
CreatableandRetrievable, we should ensure these base classes are also updated for Pydantic v2 compatibility.✅ Verification successful
Base classes are compatible with Pydantic v2
The parent classes (
CreatableandRetrievable) inherit fromResource, which uses a minimal Pydantic implementation that's compatible with both v1 and v2. No version-specific patterns were found that would require migration.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if base classes have been migrated to Pydantic v2 patterns echo "Checking base classes for Pydantic v2 compatibility..." # Look for old Config class pattern in base classes rg -l "class Config:" "cuenca/resources/base.py" # Look for new model_config pattern in base classes rg -l "model_config = ConfigDict" "cuenca/resources/base.py"Length of output: 228
Script:
#!/bin/bash # First, find where Creatable and Retrievable are defined echo "Searching for base class definitions..." ast-grep --pattern 'class Creatable' ast-grep --pattern 'class Retrievable' # Also search for imports to find the correct file echo -e "\nSearching for import statements..." rg "from.*import.*(Creatable|Retrievable)" -g '*.py'Length of output: 2681
Script:
#!/bin/bash echo "Checking base.py for Pydantic patterns..." # Check imports echo "Checking imports..." rg "from pydantic" cuenca/resources/base.py # Check class implementations echo -e "\nChecking class implementations..." rg "class (Creatable|Retrievable)" -A 10 cuenca/resources/base.py # Look for any Config classes or model_config echo -e "\nChecking Pydantic configuration..." rg "class Config|model_config" cuenca/resources/base.pyLength of output: 1180
Script:
#!/bin/bash # Check Resource class implementation echo "Checking Resource class implementation..." ast-grep --pattern 'class Resource' # Also check for any Resource class references with grep for context echo -e "\nChecking Resource class with context..." rg "class Resource" -A 15 cuenca/resources/base.pyLength of output: 761
cuenca/resources/otps.py (1)
12-19: LGTM! Pydantic v2 model configuration properly implemented.The migration from
Configclass tomodel_configusingConfigDictand the rename ofschema_extratojson_schema_extrafollows Pydantic v2's best practices.cuenca/resources/login_tokens.py (2)
11-13: LGTM! Pydantic v2 model configuration properly implemented.The migration from
Configclass tomodel_configusingConfigDictand the rename ofschema_extratojson_schema_extrafollows Pydantic v2's best practices.
1-1: Verify removal of Python 3.8 specific code.Let's ensure there are no remaining Python 3.8 specific code patterns or version checks:
✅ Verification successful
No Python 3.8 specific code found in the codebase
All checks for Python 3.8 specific features (version checks, walrus operator, positional-only parameters, and f-string = specifier) came back negative. The f-string usages found in jwt.py are standard string formatting available since Python 3.6.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for Python 3.8 specific code patterns # Check for Python version checks echo "Checking for Python version checks..." rg -l "python_version.*3\.8|python_requires.*3\.8|pyversion.*3\.8" . # Check for Python 3.8 specific syntax in setup files echo "Checking setup files for Python 3.8 references..." rg -l "3\.8" setup.* pyproject.* tox.ini # Check for Python 3.8 in CI configurations echo "Checking CI configurations..." rg -l "3\.8" .github/workflows/*Length of output: 623
Script:
#!/bin/bash # Check for Python 3.8 specific syntax patterns # Check for walrus operator (:=) echo "Checking for walrus operator usage..." rg -l ":=" --type py . # Check for positional-only parameters (function definitions with /) echo "Checking for positional-only parameters..." ast-grep --pattern 'def $_($_ / $_)' # Check for f-string = specifier echo "Checking for f-string = specifier..." rg -l "f[\"'].*=.*[\"']" --type py .Length of output: 383
Script:
#!/bin/bash # Examine f-string usage in jwt.py echo "Checking f-string usage context in jwt.py..." rg "f[\"'].*=.*[\"']" -A 2 -B 2 ./cuenca/jwt.pyLength of output: 418
cuenca/resources/whatsapp_transfers.py (1)
17-17: LGTM! Good improvement in type clarity.The changes improve code clarity by explicitly initializing optional fields with
None. This aligns well with Python type hinting best practices.Also applies to: 20-22
cuenca/resources/resources.py (1)
19-26: LGTM! Consider monitoring performance impact.The change from concurrent to sequential processing is well-justified:
- Fixes VCR test reliability issues
- Small URI lists minimize performance impact
Consider monitoring the performance impact in production, especially if the URI list sizes grow. You might want to add metrics for:
- Average URI list size
- Processing time for retrieve_uris calls
cuenca/resources/limited_wallets.py (1)
24-26:⚠️ Potential issueBreaking Change:
allowed_curpis now requiredThe
createmethod signature has changed to makeallowed_curpmandatory. This is a breaking change that will affect existing code that doesn't provide this parameter.Let's verify the impact:
cuenca/resources/balance_entries.py (1)
11-11: LGTM: Improved type safety with UnionThe change from
TypeVartoUnionprovides better type safety and is more explicit about the allowed types. This change is compatible with Python 3.9+.setup.py (2)
31-35: LGTM! Python version updates align with PR objectives.The classifiers correctly reflect the dropped support for Python 3.8 and addition of support through Python 3.13.
24-24: Verify the use of pre-release dependency.The dependency on
cuenca-validations==2.0.0.dev9is a pre-release version. This could introduce instability in production. Consider:
- Using a stable release if available
- Adding a comment explaining why a pre-release version is necessary
- Planning for an update when the stable version is released
Also applies to: 26-28
✅ Verification successful
Pre-release dependency update is acceptable
The change from
cuenca-validations==2.0.0.dev8todev9is an incremental update within the same pre-release series that's already in use. This maintains the existing development track without introducing additional risk.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if a stable version exists curl -s https://pypi.org/pypi/cuenca-validations/json | jq -r '.releases | keys[]' | sort -V | tail -n 5Length of output: 161
Script:
#!/bin/bash # Check current requirements and any migration notes echo "Previous version in requirements:" rg "cuenca-validations" -B1 -A1 --no-filename echo -e "\nChecking for migration/upgrade notes:" fd "CHANGELOG|UPGRADING|MIGRATION" --type f cat $(fd "CHANGELOG|UPGRADING|MIGRATION" --type f) 2>/dev/null || trueLength of output: 786
cuenca/resources/card_activations.py (2)
18-18: LGTM! Explicit None default follows Pydantic v2 best practices.The addition of
= Nonemakes the optional nature of the field more explicit.
46-46: LGTM! Correct migration to Pydantic v2 API.The change from
req.dict()toreq.model_dump()correctly follows Pydantic v2 migration guidelines.cuenca/resources/kyc_validations.py (2)
13-15: LGTM! Explicit None defaults enhance clarity.The addition of
= Noneto optional fields improves code clarity and follows Pydantic v2 best practices.
Line range hint
16-27: LGTM! Correct migration to Pydantic v2 configuration.The change from
Configclass tomodel_config = ConfigDict(...)follows Pydantic v2 migration guidelines.cuenca/resources/user_lists_validation.py (2)
42-42: LGTM! Correct migration to Pydantic v2 API.The change from
req.dict()toreq.model_dump()correctly follows Pydantic v2 migration guidelines.
38-38: Verify type compatibility between Clabe and Optional[str].The account_number is defined as
Optional[str]but cast toClabe. Ensure that:
- The Clabe type is compatible with the field definition
- The cast handles None values correctly
✅ Verification successful
Type casting to Clabe is safe and compatible
The cast is safe because:
- Clabe is a Pydantic model with underlying str field
- Optional values are properly handled through Pydantic
- Similar pattern is used elsewhere in the codebase (users.py)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check Clabe type definition and str compatibility rg -A 5 "class Clabe" || ast-grep --pattern 'class Clabe { $$$ }'Length of output: 318
Script:
#!/bin/bash # Check Clabe initialization and parent classes ast-grep --pattern 'def __init__($$$) { $$$ }' cuenca/resources/clabes.py # Check for Optional[Clabe] usage patterns rg "Optional\[Clabe\]" # Check parent class definitions rg -A 3 "class (Creatable|Queryable|Retrievable)"Length of output: 912
Script:
#!/bin/bash # Check Resource class definition and type handling rg -A 10 "class Resource" cuenca/resources/base.py # Check if there's any type conversion in Resource ast-grep --pattern 'class Resource { $$$ def __init__($$$) { $$$ } $$$ }'Length of output: 385
cuenca/resources/user_logins.py (1)
5-5: LGTM! Required import for Pydantic v2.The explicit import of
ConfigDictfrom pydantic is correct for v2 compatibility.cuenca/resources/savings.py (1)
20-21: LGTM! Improved type safety with explicit defaults.The addition of default
Nonevalues for optional fields improves type safety and clarity.tests/resources/test_endpoints.py (1)
46-46: Verify URL normalization behavior change.The assertion now expects a trailing slash. This might be related to Pydantic v2's URL handling changes.
Run this script to check URL handling across the codebase:
✅ Verification successful
URL assertion change is isolated and aligned with Pydantic's URL handling
The trailing slash assertion is the only URL assertion in the test suite, and the codebase consistently uses Pydantic's URL validation types (HttpUrl, HttpUrlString, AnyUrlString) which handle URL normalization. This change appears to be correctly adapting to Pydantic's URL handling behavior.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for URL assertions and URL field usage rg -l 'assert.*url.*=.*["\']https?://' | while read -r file; do echo "=== $file ===" rg 'assert.*url.*=.*["\']https?://' "$file" done # Check URL field definitions ast-grep --pattern 'url: HttpUrlString'Length of output: 116
Script:
#!/bin/bash # Search for URL assertions in test files echo "=== URL Assertions ===" rg "assert.*url.*=.*'https?://" || rg 'assert.*url.*="https?://' # Search for URL-related field definitions echo -e "\n=== URL Field Definitions ===" rg "url.*=.*Field|url:.*str|url:.*Url" # Check for Pydantic models and configs echo -e "\n=== Pydantic Models ===" rg "class.*\(.*BaseModel.*\)" -A 5Length of output: 1323
cuenca/resources/kyc_verifications.py (3)
10-10: LGTM! Updated to Pydantic v2 configuration style.The model configuration has been correctly updated to use Pydantic v2's
ConfigDict.Also applies to: 26-39
21-22: LGTM! Improved type safety with explicit defaults.Adding default
Nonevalues for optional fields improves type safety and clarity.
49-49:⚠️ Potential issueBreaking change: curp parameter is now required.
The
curpparameter in theupdatemethod is no longer optional. This is a breaking change that needs to be documented.Run this script to check for existing usage:
Also, update the
dict()call to usemodel_dump():- return cast('KYCVerification', cls._update(id=kyc_id, **req.dict())) + return cast('KYCVerification', cls._update(id=kyc_id, **req.model_dump()))Also applies to: 52-52
✅ Verification successful
Breaking change is safe to proceed, but needs documentation
The only existing usage of
KYCVerification.update()already provides thecurpparameter, making this a safe change. However:
- Document this breaking change in the changelog
- Update the method's docstring to clearly indicate that
curpis required- Consider implementing the suggested Pydantic v2 migration (
dict()→model_dump())🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for KYCVerification.update calls rg -A 3 'KYCVerification\.update.*\(' | grep -v 'def update'Length of output: 396
Script:
#!/bin/bash # Check the update method implementation and test file content echo "=== KYCVerification update method ===" ast-grep --pattern 'def update($$$) { $$$ }' cuenca/resources/kyc_verifications.py echo -e "\n=== Test file content ===" cat tests/resources/test_kyc_verifications.py echo -e "\n=== Checking for indirect usages ===" rg -l "KYCVerification" | xargs rg "update"Length of output: 1495
cuenca/resources/arpc.py (2)
27-28: LGTM! Explicit initialization of optional fields.The explicit
Nonedefaults for optional fields align with Pydantic v2's improved schema clarity.
54-54: Verify type casting and model serialization changes.The changes align with Pydantic v2 migration:
- Using
model_dump()instead ofdict()follows v2 recommendations- Explicit type casting of
track_data_methodenhances type safetyLet's verify other usages of the deprecated
dict()method:Also applies to: 56-56
✅ Verification successful
Type casting and serialization changes are consistent with codebase patterns
The changes in
arpc.pyalign with:
- Ongoing Pydantic v2 migration (both
dict()andmodel_dump()methods are present across the codebase)- Established type casting patterns used throughout the repository for type safety
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for remaining dict() calls that might need migration rg --type python 'req\.dict\(' .Length of output: 65
Script:
#!/bin/bash # Search for dict() method calls in Python files rg '\.dict\(' --glob '*.py' # Search for model_dump() usage rg '\.model_dump\(' --glob '*.py' # Search for similar type casting patterns rg 'cast\([^,]+,' --glob '*.py'Length of output: 7825
cuenca/resources/verifications.py (3)
19-21: LGTM! Enhanced field documentation.Good use of Pydantic's
Fieldwith description to improve API documentation.
25-34: LGTM! Updated to Pydantic v2 configuration pattern.Correctly migrated from
Configclass tomodel_configusingConfigDict, following Pydantic v2 patterns.
48-50: LGTM! Updated serialization method.Properly migrated from
dict()tomodel_dump()following Pydantic v2 recommendations.Also applies to: 62-62
cuenca/resources/files.py (2)
4-9: LGTM! Improved type imports.Good organization of imports from cuenca_validations.types.
48-48: LGTM! Enhanced type safety.Good use of explicit type casting for the extension parameter.
cuenca/resources/card_validations.py (3)
21-25: LGTM! Explicit initialization of optional fields.Good practice to explicitly set
Nonedefaults for optional boolean fields.
55-55: LGTM! Updated serialization method.Properly migrated from
dict()tomodel_dump()following Pydantic v2 recommendations.
40-40: Verify the type change impact.The type change from
Optional[str]toOptional[bool]forpin_attempts_exceededis a breaking change.Let's verify the impact:
cuenca/resources/sessions.py (2)
10-16: Well-documented URL field compatibility solution!Great job documenting the rationale for using
AnyUrlString. The comment clearly explains the compatibility issue with Pydantic v2's URL fields and the solution.
27-29: LGTM: Field type updatesThe transition from
AnyUrltoAnyUrlStringfor URL fields is correctly implemented, maintaining string compatibility while ensuring URL validation.cuenca/resources/api_keys.py (1)
Line range hint
16-29: LGTM: Clean Pydantic v2 migrationGood updates:
- Added explicit
Nonedefaults to optional fields- Correctly implemented
model_configusingConfigDictcuenca/resources/platforms.py (2)
15-34: Excellent field documentation!Great job adding descriptive Field annotations. This significantly improves API documentation and code clarity.
76-78: LGTM: Pydantic v2 method updatesCorrect usage of
model_dump()instead ofdict(), which is the recommended approach in Pydantic v2.cuenca/resources/transfers.py (2)
29-29: LGTM: Explicit optional field initializationGood practice adding explicit
Nonedefault to the optionaltracking_keyfield.
66-66: LGTM: Enhanced type safetyGood use of
cast(Clabe, account_number)to ensure type safety when creating the transfer request.cuenca/resources/cards.py (1)
24-24: LGTM! Clear optional field definitions.The changes make the optional nature of
user_idandpinfields more explicit by adding default values, which aligns well with Pydantic v2's best practices.Also applies to: 29-29
cuenca/resources/endpoints.py (3)
13-19: Well-documented URL field type compatibility fix.The introduction of
HttpUrlStringtype alias effectively addresses the compatibility issue with Pydantic v2's URL handling while maintaining type safety. The comments clearly explain the rationale.
25-40: Enhanced field definitions with clear descriptions.The Field definitions now include comprehensive descriptions, improving the API documentation. This is a good practice that makes the code more maintainable.
Line range hint
41-60: Updated model configuration to Pydantic v2 style.The transition from
Configclass tomodel_configwithConfigDictaligns with Pydantic v2's new configuration pattern.cuenca/resources/curp_validations.py (3)
21-55: Comprehensive field definitions with clear descriptions.All fields have been updated with proper descriptions using Pydantic's Field, which significantly improves the API documentation and schema generation.
Line range hint
56-76: Updated model configuration to Pydantic v2 style.The transition from
Configclass tomodel_configwithConfigDictfollows Pydantic v2's best practices.
98-104: Updated to use model_dump() instead of dict().The change from
req.dict()toreq.model_dump()is essential for Pydantic v2 compatibility asdict()is deprecated.tests/resources/test_transfers.py (1)
132-132: Updated test assertion for Pydantic v2 error message.The error message assertion has been correctly updated to match Pydantic v2's new validation error format.
cuenca/resources/base.py (2)
27-27: Type annotation broadened for better flexibilityThe change from
Dict[str, Union[str, int]]toDict[str, Any]provides better flexibility for handling various field types. This is a good change as it reduces potential type conflicts while maintaining type safety through Pydantic's validation.
82-82: Good practice: Explicit initialization of optional fieldAdding
= Nonedefault value todeactivated_atmakes the optional nature of the field explicit and follows Pydantic's best practices.cuenca/resources/users.py (3)
33-38: Enhanced field documentation with descriptive metadataGood addition of field descriptions using
Field. This improves API documentation and code maintainability.
Line range hint
77-101: Updated to Pydantic v2's model configurationThe change from
Configclass tomodel_configwithConfigDictaligns with Pydantic v2's new configuration pattern. The example schema is well-documented.
133-133: Updated to use model_dump() instead of dict()Consistent update to use
model_dump()instead ofdict()in both create and update methods, aligning with Pydantic v2's API.Also applies to: 174-174
requirements-test.txt (1)
1-12: Test dependencies properly updatedAll test dependencies have been updated to stable versions that support Python 3.9+. This is appropriate given the decision to drop Python 3.8 support.
.github/workflows/test.yml (1)
23-23: Verify Pydantic v2 compatibility with all Python versions.The test matrix has been updated to include Python versions 3.9 through 3.13. Since this PR migrates to Pydantic v2, ensure that all these Python versions are supported by Pydantic v2 and its dependencies.
✅ Verification successful
Python versions in test matrix are compatible with Pydantic v2
The test matrix (Python 3.9-3.13) aligns with both the project's Python requirement (>=3.9) and Pydantic v2's supported versions (3.7+). All specified versions are supported.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check Pydantic v2's Python version compatibility # and verify dependencies in requirements.txt # Check if requirements.txt exists and contains Pydantic if [ -f "requirements.txt" ]; then echo "Checking Pydantic version in requirements.txt:" grep -i "pydantic" requirements.txt fi # Check if setup.py exists and contains Pydantic if [ -f "setup.py" ]; then echo "Checking Pydantic version in setup.py:" grep -i "pydantic" setup.py fi # Check for any Python version constraints in project files echo "Checking Python version constraints in project files:" rg "python_requires|python_version" setup.py pyproject.tomlLength of output: 646
Script:
#!/bin/bash # Search for Pydantic core package in all dependency files echo "Searching for Pydantic core package:" fd -e txt -e toml -e cfg -e ini -e yaml -e yml | xargs grep -i "pydantic[^-]" 2>/dev/null || true # Check for any additional requirements files echo -e "\nChecking for additional requirements files:" fd "requirements" -e txt -e inLength of output: 342
Script:
#!/bin/bash # Check test requirements file echo "Checking requirements-test.txt for Pydantic:" cat requirements-test.txt | grep -i "pydantic" # Look for any Python version constraints echo -e "\nChecking for Python version constraints:" cat requirements-test.txt | grep -i "python"Length of output: 287
requirements.txt
Outdated
| requests==2.32.3 | ||
| cuenca-validations==2.0.0.dev8 | ||
| pydantic-extra-types==2.10.* |
There was a problem hiding this comment.
💡 Codebase verification
Revert to stable version of cuenca-validations
The current development version (2.0.0.dev8) is a major version ahead of the latest stable release (0.11.33). Using development versions in production dependencies can lead to instability. Consider:
- Using the latest stable version:
cuenca-validations==0.11.33 - Or waiting for the stable 2.0.0 release if new features are required
🔗 Analysis chain
Verify stability of development dependency
While the dependency updates are good, using cuenca-validations==2.0.0.dev8 in production could be risky. Consider waiting for a stable release.
Let's check the latest stable version:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest stable version of cuenca-validations
curl -s https://pypi.org/pypi/cuenca-validations/json | jq -r '.releases | keys[]' | grep -v "dev" | sort -V | tail -n 1
Length of output: 128
…p-python and codecov-action
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
cuenca/resources/sessions.py (1)
42-43:⚠️ Potential issueFix cast() usage in method parameters
Using
cast()in method parameter defaults is discouraged as it's executed at function definition time. This could lead to unexpected behavior.Apply this diff to fix the issue:
@classmethod def create( cls, user_id: str, type: SessionType, - success_url=cast(Optional[AnyUrl], success_url), - failure_url=cast(Optional[AnyUrl], failure_url), + success_url: Optional[AnyUrl] = None, + failure_url: Optional[AnyUrl] = None, *, session: http.Session = http.session, ) -> 'Session':🧰 Tools
🪛 Ruff (0.8.2)
42-42: Do not perform function call
castin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
43-43: Do not perform function call
castin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
🧹 Nitpick comments (5)
tests/resources/test_sessions.py (2)
10-10: Consider usingtyping.Dictfor better type safety with Pydantic.While Python 3.9+ allows using built-in types as type hints, Pydantic v2 recommends using
typing.Dictfor better type safety and explicit key/value types. Consider updating to:- def test_session_create(curp_validation_request: dict, user_request: dict): + def test_session_create(curp_validation_request: Dict[str, Any], user_request: Dict[str, Any]):
Line range hint
10-41: Consider adding Pydantic v2 specific test cases.Since this PR migrates to Pydantic v2, consider adding test cases for new v2 features like:
- Strict type validation
- Custom serializers
- New model config options
tests/conftest.py (2)
33-33: Consider usingtyping.Dictconsistently across fixtures.For consistency with Pydantic v2 and better type safety, consider using explicit
typing.Dictwith key/value types:- def curp_validation_request() -> dict: + def curp_validation_request() -> Dict[str, Union[str, dt.date, State, Country, Gender]]: - def user_request() -> dict: + def user_request() -> Dict[str, Union[str, Dict[str, Union[str, State, Country]]]]: - def user_lists_request() -> dict: + def user_lists_request() -> Dict[str, str]:Also applies to: 47-47, 66-66
Line range hint
33-74: Consider using Pydantic models for test data.Since the project uses Pydantic, consider defining test data using Pydantic models instead of raw dictionaries. This would:
- Ensure type safety at the model level
- Validate test data structure
- Make test data intentions clearer
- Leverage Pydantic v2's improved performance
Example:
from pydantic import BaseModel class CurpValidationRequest(BaseModel): names: str first_surname: str second_surname: str date_of_birth: dt.date state_of_birth: State country_of_birth: Country gender: Gender @pytest.fixture def curp_validation_request() -> Dict[str, Any]: return CurpValidationRequest( names='José', first_surname='López', # ... other fields ... ).model_dump()cuenca/resources/user_credentials.py (1)
29-29: Correct migration to Pydantic v2 with proper type safety!The changes from
dict()tomodel_dump()are correctly implemented while maintaining type safety through proper casting.Consider using
TypeAliasfor the repeated cast type to improve maintainability:+from typing import TypeAlias + +UserCredentialType: TypeAlias = 'UserCredential' + class UserCredential(Creatable, Updateable): # ... @classmethod def create( # ... ) -> 'UserCredential': req = UserCredentialRequest(password=password, user_id=user_id) - return cast('UserCredential', cls._create(**req.model_dump(), session=session)) + return cast(UserCredentialType, cls._create(**req.model_dump(), session=session))Also applies to: 47-47
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
.github/workflows/release.yml(1 hunks).github/workflows/test.yml(3 hunks)cuenca/resources/api_keys.py(4 hunks)cuenca/resources/base.py(8 hunks)cuenca/resources/card_activations.py(3 hunks)cuenca/resources/card_transactions.py(2 hunks)cuenca/resources/cards.py(3 hunks)cuenca/resources/endpoints.py(5 hunks)cuenca/resources/file_batches.py(2 hunks)cuenca/resources/files.py(2 hunks)cuenca/resources/kyc_validations.py(4 hunks)cuenca/resources/kyc_verifications.py(4 hunks)cuenca/resources/limited_wallets.py(2 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(4 hunks)cuenca/resources/user_credentials.py(2 hunks)cuenca/resources/user_logins.py(3 hunks)cuenca/resources/users.py(9 hunks)cuenca/resources/wallet_transactions.py(1 hunks)cuenca/resources/webhooks.py(1 hunks)requirements-test.txt(1 hunks)requirements.txt(1 hunks)tests/conftest.py(3 hunks)tests/resources/cassettes/test_card_activation.yaml(1 hunks)tests/resources/test_card_activations.py(1 hunks)tests/resources/test_commissions.py(2 hunks)tests/resources/test_otps.py(1 hunks)tests/resources/test_sessions.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (15)
- cuenca/resources/webhooks.py
- .github/workflows/release.yml
- cuenca/resources/files.py
- cuenca/resources/cards.py
- requirements.txt
- cuenca/resources/kyc_validations.py
- cuenca/resources/card_transactions.py
- cuenca/resources/api_keys.py
- cuenca/resources/savings.py
- cuenca/resources/kyc_verifications.py
- cuenca/resources/transfers.py
- cuenca/resources/file_batches.py
- cuenca/resources/user_logins.py
- cuenca/resources/users.py
- cuenca/resources/base.py
🧰 Additional context used
🪛 GitHub Actions: test
requirements-test.txt
[error] 11-12: Dependency conflict detected between packages on lines 11 and 12. Resolution is impossible with the current package versions.
🪛 Ruff (0.8.2)
cuenca/resources/sessions.py
42-42: Do not perform function call cast in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
43-43: Do not perform function call cast in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🔇 Additional comments (24)
cuenca/resources/limited_wallets.py (3)
40-40: LGTM! Correct usage of Pydantic v2 API.The change from
dict()tomodel_dump()correctly implements Pydantic v2's serialization API.
18-18: LGTM! Note the breaking change.The default value addition aligns with Pydantic v2's best practices for optional fields. However, this is a breaking change that might affect existing code initializing
LimitedWalletinstances.Let's verify the impact on existing initializations:
✅ Verification successful
Breaking change verified - no impact on existing code
The addition of
Noneas default value forallowed_rfcis safe. The only usage found in the codebase explicitly sets this parameter during creation, and no instances were found where the parameter is omitted.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for LimitedWallet instantiations that might be affected rg -A 3 "LimitedWallet\(" --type pyLength of output: 323
Script:
#!/bin/bash # Search for imports of LimitedWallet rg "from cuenca.resources.limited_wallets import LimitedWallet|from cuenca.resources import LimitedWallet" --type py # Search for LimitedWallet in test files rg "LimitedWallet" tests/ --type py || echo "No tests directory found"Length of output: 417
24-25: LGTM! Note the breaking change in method signature.Making
allowed_curpmandatory improves type safety and aligns with the business requirement that a limited wallet must be tied to a specific person's CURP. However, this is a breaking change that will require updates to existing code calling this method.Let's verify the impact on existing method calls:
✅ Verification successful
Breaking change verification completed - minimal impact
The mandatory
allowed_curpparameter inLimitedWallet.create()has minimal impact on the codebase:
- The only direct usage in tests already provides the required parameter
- No other calls to this method found in the codebase
- No classes inherit from or compose with
LimitedWallet🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for create method calls that might be affected rg -A 3 "LimitedWallet\.create\(" --type pyLength of output: 378
Script:
#!/bin/bash # Check for classes inheriting from LimitedWallet ast-grep --pattern 'class $_ (LimitedWallet)' # Check for any other references to LimitedWallet rg "LimitedWallet" --type py -C 2Length of output: 3655
cuenca/resources/service_providers.py (2)
13-13: LGTM! Type annotation change aligns with modern Python.The change from
Listtolisttype annotation is appropriate for Python 3.9+ and aligns well with Pydantic v2's preferences for built-in types.
Line range hint
1-14: Verify Pydantic v2 compatibility of dependenciesPlease ensure that the imported components are compatible with Pydantic v2:
- The
cuenca_validations.types.ServiceProviderCategory- The base classes
RetrievableandQueryableLet's verify the compatibility:
✅ Verification successful
Dependencies are Pydantic v2 compatible
The codebase shows clear signs of Pydantic v2 adoption:
cuenca-validationsis at version 2.0.0.dev9- Base classes use modern Pydantic patterns with ConfigDict
- Dependencies include Pydantic v2 compatible packages
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for Pydantic v2 compatibility in related files # Check ServiceProviderCategory implementation echo "Checking ServiceProviderCategory implementation:" rg -A 5 "class ServiceProviderCategory" # Check base classes for Pydantic v2 patterns echo "Checking base classes for Pydantic v2 patterns:" rg -A 10 "class (Retrievable|Queryable)" # Look for Pydantic imports and usage echo "Checking Pydantic usage:" rg "from pydantic"Length of output: 3076
Script:
#!/bin/bash # Check Resource class implementation echo "Checking Resource class implementation:" ast-grep --pattern 'class Resource($_) { $$$ }' # Check for ServiceProviderCategory usage echo "Checking ServiceProviderCategory usage:" rg -A 5 "ServiceProviderCategory" # Check package dependencies echo "Checking package dependencies:" cat pyproject.toml 2>/dev/null || cat setup.py 2>/dev/null || echo "No dependency file found"Length of output: 2618
tests/conftest.py (1)
Line range hint
1-41: Verify Pydantic v2 compatibility in test files.Let's check for any Pydantic v1 specific features that need updates:
✅ Verification successful
No Pydantic v2 migration changes needed in test files
The test files, including
conftest.py, only use type imports fromcuenca_validationsand don't contain any Pydantic v1-specific features that would need updates for v2 compatibility.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Pydantic v1 specific features that might need updates # Check for deprecated v1 features echo "Checking for deprecated Pydantic v1 features..." rg -l 'validator|root_validator|Config|@validator|@root_validator|parse_obj|parse_raw|parse_file|json|schema|construct|validate_model' tests/ # Check for model configurations echo "Checking for model configurations that might need updates..." rg -l 'class Config:|from_orm|orm_mode|allow_mutation|allow_population_by_field_name' tests/ # Check for type annotations that might need updates echo "Checking type annotations..." rg -l 'from typing import.*Dict|from typing import.*Optional|from typing import.*List|from typing import.*Union' tests/Length of output: 7427
Script:
#!/bin/bash # Check actual Pydantic usage in test files echo "Checking validator usage..." rg -C 3 '@validator|@root_validator|validator\(' 'tests/test_cuenca.py' 'tests/resources/test_endpoints.py' 'tests/resources/test_verifications.py' 'tests/resources/test_user_lists_validation.py' 'tests/resources/test_cards.py' echo -e "\nChecking model configurations..." rg -C 3 'class Config:|from_orm|orm_mode' 'tests/test_cuenca.py' 'tests/resources/test_endpoints.py' 'tests/resources/test_verifications.py' 'tests/resources/test_user_lists_validation.py' 'tests/resources/test_cards.py' echo -e "\nChecking parse methods..." rg -C 3 'parse_obj|parse_raw|parse_file|validate_model' 'tests/test_cuenca.py' 'tests/resources/test_endpoints.py' 'tests/resources/test_verifications.py' 'tests/resources/test_user_lists_validation.py' 'tests/resources/test_cards.py'Length of output: 872
Script:
#!/bin/bash # Check for model usage and imports echo "Checking for model imports and usage..." rg -C 3 'from.*import.*BaseModel|class.*\(BaseModel\)|model.*validate|model.*dict\(\)' tests/ echo -e "\nChecking for validation related code..." rg -C 3 'validate|validation' tests/test_cuenca.py tests/resources/test_verifications.py tests/resources/test_user_lists_validation.py echo -e "\nChecking imports from cuenca_validations..." rg -C 3 'from cuenca_validations' tests/Length of output: 8436
tests/resources/test_otps.py (1)
28-28: Good improvement in type checking!The change from
type(x) == strtoisinstance(x, str)is a better practice as it:
- Handles inheritance correctly
- Is more idiomatic Python
- Is more maintainable
cuenca/resources/wallet_transactions.py (1)
38-38: Correct migration to Pydantic v2!The change from
dict()tomodel_dump()aligns with Pydantic v2's API changes.Let's verify the Pydantic version in use:
✅ Verification successful
Pydantic v2 migration verified successfully!
The codebase consistently uses Pydantic v2 features (ConfigDict, model_dump()) and dependencies (pydantic-extra-types 2.10.*). The change from
dict()tomodel_dump()is correct and aligned with the rest of the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check Pydantic version in requirements files rg -l "pydantic" | grep -i "requirements" | xargs catLength of output: 129
Script:
#!/bin/bash # Check for direct pydantic imports and version constraints rg "from pydantic" -A 1 rg "import pydantic" rg "pydantic[>=<]" --type yaml --type txt --type tomlLength of output: 2253
tests/resources/test_card_activations.py (1)
11-14: Test data updates look good!The updates to test values (card number, CVV, user_id) maintain the test's integrity while using new test data.
Also applies to: 18-18, 21-21
cuenca/resources/resources.py (2)
7-7: Type hint update aligns with modern Python practicesThe change from
Dicttodictaligns with PEP 585 which made built-in collections support generics.
18-26: Sequential processing improves test reliabilityGood decision to replace ThreadPoolExecutor with a list comprehension. The comment explains the rationale well - the performance impact is negligible for small URI lists, and it resolves VCR test reliability issues.
tests/resources/test_commissions.py (1)
20-20: Better type checking with isinstance()Good improvement replacing
type()withisinstance(). This change makes the tests more robust by properly supporting inheritance in type checking.Also applies to: 31-31
.github/workflows/test.yml (2)
23-23: Python version updates align with project objectivesThe updated Python version matrix correctly removes 3.8 and adds newer versions up to 3.13, aligning with the PR objective to drop Python 3.8 support.
48-48: Verify Codecov token configurationWhile updating to codecov-action@v5 is good, ensure the CODECOV_TOKEN secret is properly configured in the repository settings.
cuenca/resources/card_activations.py (2)
5-5: Improved type safety with PaymentCardNumberGood use of
PaymentCardNumberfrom pydantic_extra_types. This provides better validation and type safety for card numbers.Also applies to: 25-25
47-47: Pydantic v2 compatibility updateCorrect update from
dict()tomodel_dump()for Pydantic v2 compatibility.Let's check for any remaining dict() calls that need updating:
✅ Verification successful
All Pydantic v2 compatibility updates are complete
The codebase has been thoroughly checked and no remaining Pydantic
.dict()calls were found. All otherdict()usage in the codebase is for standard Python dictionaries or customto_dict()methods, which don't require updates.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for remaining dict() calls that might need updating rg "\.dict\(\)" --type pyLength of output: 27
Script:
#!/bin/bash # First, verify Python files are recognized echo "Python files in the repository:" fd -e py # Use ast-grep to search for dict() method calls echo -e "\nSearching for dict() method calls:" ast-grep --pattern '$_.$_.dict()' # Use ast-grep to search for model_dump() calls for comparison echo -e "\nSearching for model_dump() method calls:" ast-grep --pattern '$_.$_.model_dump()' # Try a different ripgrep pattern echo -e "\nAlternative ripgrep search:" rg "dict\(\)" --type pyLength of output: 4724
Script:
#!/bin/bash # Search for Pydantic BaseModel usage to find potential model instances echo "Files containing Pydantic BaseModel:" rg "BaseModel" --type py -l # Search for specific Pydantic model patterns echo -e "\nSearching for potential Pydantic model usage:" rg "class.*\(.*BaseModel.*\):" --type py -A 5Length of output: 742
cuenca/resources/sessions.py (3)
19-21: LGTM! Field type updates align with Pydantic v2The transition from
AnyUrltoAnyUrlStringfor URL fields is a good change that aligns with Pydantic v2 best practices.
Line range hint
22-35: LGTM! Proper migration to Pydantic v2 configurationThe transition from
Configclass tomodel_config = ConfigDict()follows the recommended Pydantic v2 pattern.
53-55: LGTM! Proper usage of model_dump()The transition from
dict()tomodel_dump()aligns with Pydantic v2's API changes.tests/resources/cassettes/test_card_activation.yaml (1)
3-4: Verify test data sensitivityThe test cassettes contain card numbers and CVV values. While these appear to be test data, please verify that no real card data is being committed.
Also applies to: 64-64
cuenca/resources/endpoints.py (4)
18-33: LGTM! Well-documented field definitionsThe addition of detailed field descriptions using
Field()improves code documentation and API schema generation. The transition toHttpUrlStringis consistent with other URL field changes.
Line range hint
34-53: LGTM! Proper Pydantic v2 configurationThe migration from
Configclass tomodel_config = ConfigDict()with comprehensive examples follows Pydantic v2 best practices.
73-75: LGTM! Consistent use of model_dump()The transition from
dict()tomodel_dump()is consistently applied across methods, aligning with Pydantic v2's API changes.Also applies to: 99-100
Line range hint
1-100: Verify Python 3.8 compatibility removalSince this PR drops Python 3.8 support, verify that all Python 3.8-specific code patterns have been removed or updated.
✅ Verification successful
Python 3.8 support properly removed
The codebase has been correctly updated to drop Python 3.8 support:
- setup.py requires Python >=3.9
- No Python 3.8-specific compatibility code or patterns found
- Type hints used in the code are compatible with Python 3.9+
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for Python 3.8-specific patterns or compatibility code rg -l "python_version.*3\.8" . rg -l "if sys\.version_info.*3\.8" .Length of output: 71
Script:
#!/bin/bash # Check for Python version requirements in setup files fd "setup.py|pyproject.toml" --exec cat {} # Check for typing-related imports and usage that might need updates rg "from typing import .*Union" -A 2 rg "from typing import .*Literal" -A 2 rg "from typing import .*Protocol" -A 2 rg "from typing import .*TypedDict" -A 2 rg "from typing_extensions import" -A 2 rg "get_(args|origin)\(" -A 2Length of output: 1877
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
cuenca/resources/kyc_validations.py (2)
Line range hint
16-27: Fix type mismatch in json_schema_extra example.The 'attemps' field in the example is defined as string '1', but the type hint is
Optional[int].Apply this fix:
'example': { 'id': 'KVNEUInh69SuKXXmK95sROwQ', 'platform_id': 'PT8UEv02zBTcymd4Kd3MO6pg', 'created_at': '2020-05-24T14:15:22Z', 'verification_id': 'string', 'files_uri': ['cuenca.com/files/id', 'cuenca.com/files/id2'], - 'attemps': '1', + 'attemps': 1, }
Line range hint
29-43: Fix mutable default argument in create method.Using a mutable default argument (empty list) can lead to unexpected behavior if the default value is shared between calls.
Apply this fix:
@classmethod def create( cls, user_id: str, force: bool = False, - documents: list[KYCFile] = [], + documents: list[KYCFile] | None = None, session: Session = global_session, ) -> 'KYCValidation': req = KYCValidationRequest( user_id=user_id, force=force, - documents=documents, + documents=documents or [], ) return cast( 'KYCValidation', cls._create(**req.model_dump(), session=session) )🧰 Tools
🪛 Ruff (0.8.2)
34-34: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
🧹 Nitpick comments (1)
cuenca/resources/sessions.py (1)
50-54: Improve type safety in the create methodWhile the migration to
model_dump()is correct, there are a few type-related improvements that could be made:
- The
# type: ignorecomments are masking potential type issues. Consider explicitly typing the URL parameters asOptional[AnyUrlString].- The
cast()usage in the return statement could be simplified since we're already working with typed objects.Consider applying this diff:
@classmethod def create( cls, user_id: str, type: SessionType, - success_url: Optional[str] = None, - failure_url: Optional[str] = None, + success_url: Optional[AnyUrlString] = None, + failure_url: Optional[AnyUrlString] = None, *, session: http.Session = http.session, ) -> 'Session': req = SessionRequest( user_id=user_id, type=type, - success_url=success_url, # type: ignore - failure_url=failure_url, # type: ignore + success_url=success_url, + failure_url=failure_url, ) - return cast( - 'Session', cls._create(session=session, **req.model_dump()) - ) + return cls._create(session=session, **req.model_dump())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cuenca/resources/endpoints.py(5 hunks)cuenca/resources/kyc_validations.py(4 hunks)cuenca/resources/sessions.py(4 hunks)cuenca/resources/transfers.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cuenca/resources/endpoints.py
- cuenca/resources/transfers.py
🧰 Additional context used
🪛 Ruff (0.8.2)
cuenca/resources/kyc_validations.py
34-34: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
🔇 Additional comments (5)
cuenca/resources/kyc_validations.py (2)
1-7: LGTM! Import changes align with Pydantic v2 and modern Python type hints.The changes follow best practices by:
- Using
ConfigDictfrom Pydantic v2- Using built-in collection types for type hints (PEP 585)
13-15: LGTM! Explicit None defaults improve clarity.The changes follow Pydantic v2 best practices by making Optional fields' defaults explicit.
cuenca/resources/sessions.py (3)
4-5: LGTM! Correct imports for Pydantic v2The imports have been correctly updated to use the new Pydantic v2 patterns with
ConfigDictandAnyUrlString.
19-21: LGTM! Correct field type updates for Pydantic v2The field types have been properly updated to use
AnyUrlStringinstead ofAnyUrl, which is the recommended approach in Pydantic v2.
Line range hint
22-35: LGTM! Correct model configuration for Pydantic v2The model configuration has been properly updated to use
model_config = ConfigDict()instead of the v1Configclass pattern.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cuenca/version.py(1 hunks)requirements.txt(1 hunks)setup.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cuenca/version.py
- requirements.txt
🔇 Additional comments (1)
setup.py (1)
24-24: LGTM: Python version requirement aligns with PR objectivesThe update to require Python >=3.9 correctly implements the stated objective to drop Python 3.8 support.
| 'dataclasses>=0.7;python_version<"3.8"', | ||
| 'cuenca-validations>= 0.11.3,<0.12.0', | ||
| 'requests>=2.32.3', | ||
| 'cuenca-validations==2.0.0.dev12', |
There was a problem hiding this comment.
Critical: Avoid using development version in production
The dependency cuenca-validations==2.0.0.dev12 is a development version, which is not suitable for production use as it may contain unstable features or breaking changes.
Consider:
- Waiting for a stable release of cuenca-validations 2.0.0
- Or pin to the latest stable version that supports Pydantic v2
| "Programming Language :: Python :: 3.9", | ||
| "Programming Language :: Python :: 3.10", | ||
| "Programming Language :: Python :: 3.11", | ||
| "Programming Language :: Python :: 3.12", | ||
| "Programming Language :: Python :: 3.13", |
There was a problem hiding this comment.
💡 Codebase verification
Remove Python 3.13 classifier and update dependencies
The Python 3.13 classifier should be removed due to dependency compatibility issues:
cuenca-validationsonly supports Python up to 3.8, which conflicts with your minimum Python requirement of 3.9requestspackage hasn't officially added Python 3.13 support yet
Additionally:
- Consider upgrading
cuenca-validationsto a version that supports Python 3.9+ or replace it with an alternative - Wait for Python 3.13 to be officially released and for core dependencies to add support before adding the 3.13 classifier
🔗 Analysis chain
Verify Python 3.13 compatibility
While it's good to be forward-looking, Python 3.13 is still in development. Please ensure that:
- All dependencies support Python 3.13
- CI tests pass on Python 3.13
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if key dependencies support Python 3.13
curl -s https://pypi.org/pypi/pydantic-extra-types/json | jq '.info.classifiers[] | select(contains("Programming Language :: Python :: 3.13"))'
curl -s https://pypi.org/pypi/requests/json | jq '.info.classifiers[] | select(contains("Programming Language :: Python :: 3.13"))'
Length of output: 320
Script:
#!/bin/bash
# Check CI configuration for Python versions
fd -e yml -e yaml . .github/workflows --exec cat {} \; | grep -A 5 "python-version"
# Check dependencies in setup.py
cat setup.py | grep -A 10 "install_requires"
Length of output: 1530
Script:
#!/bin/bash
# Check Python support for cuenca-validations
curl -s https://pypi.org/pypi/cuenca-validations/json | jq '.info.classifiers[] | select(contains("Programming Language :: Python"))'
Length of output: 293
cuenca/resources/file_batches.py
Outdated
| req = FileBatchUploadRequest(files=files, user_id=user_id) | ||
| return cast('FileBatch', cls._create(session=session, **req.dict())) | ||
| req = FileBatchUploadRequest( | ||
| files=cast(List[FileRequest], files), user_id=user_id |
There was a problem hiding this comment.
porque es necesario hacer este cast?
Description:
This PR migrates the project to Pydantic v2 and drops support for Python 3.8
Key Changes:
Breaking Changes:
Summary by CodeRabbit
Release Notes: Cuenca SDK v2.0.0.dev4
Overview
This release introduces additional updates to the Cuenca SDK, including further enhancements to Python version compatibility, additional dependency upgrades, and refined type handling.
Key Changes
Python Version Support
Dependency Updates
requestsandcuenca-validations.pydantic-extra-types.Type Handling
Listto built-inlisttypes.Fieldfor better clarity and documentation.Development Dependencies
pytest,black, and others.Bug Fixes
Migration Recommendations