Update/pydantic v2 #410
Update/pydantic v2 #410
Conversation
WalkthroughThe 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 of the project, including GitHub Actions workflows, Python version updates, dependency management, and significant refactoring of resource classes. Key modifications include upgrading Python support from version 3.8 to versions 3.9-3.13, updating dependencies, and transitioning to Pydantic v2 conventions. The project now uses more modern type annotations, replaces The update also involves standardizing attribute initializations, adding field descriptions, and improving configuration management for Pydantic models. These changes aim to enhance type safety, improve documentation, and align the library with contemporary 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 1106 -5
=========================================
- Hits 1111 1106 -5
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.
setup.py
Outdated
| '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?
…in ApiKey, LoginToken, Otp, and Session classes
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
tests/resources/test_api_keys.py (2)
Line range hint
64-82: Update serialization methods for Pydantic v2 compatibilityThe
to_dict()method and direct dictionary comparison should be updated to use Pydantic v2's model_dump() method.Here's how to update the test:
- api_key_dict = dict( + api_key_dict = api_key.model_dump(exclude_unset=True) - id='12345', - secret='********', - created_at=date, - deactivated_at=None, - updated_at=date, - user_id=None, - ) - assert api_key_dict == api_key.to_dict() + assert api_key_dict == { + 'id': '12345', + 'secret': '********', + 'created_at': date, + 'deactivated_at': None, + 'updated_at': date, + 'user_id': None, + }🧰 Tools
🪛 Gitleaks (8.21.2)
18-18: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Line range hint
85-106: Update _from_dict to use Pydantic's model_validateThe
_from_dictmethod should be updated to use Pydantic v2's model_validate method.Here's how to update the test:
def test_api_key_from_dict(): api_keys_dict = dict( id='123455', secret='*********', created_at=dt.datetime.utcnow(), deactivated_at=None, extra_field_1='not necessary', extra_field_2=12345, updated_at=dt.datetime.utcnow(), metadata=None, user_id=None, ) - api_key = ApiKey._from_dict(api_keys_dict) + api_key = ApiKey.model_validate(api_keys_dict) assert not hasattr(api_key, 'extra_field_1') assert not hasattr(api_key, 'extra_field_2') assert api_key.id is not None🧰 Tools
🪛 Gitleaks (8.21.2)
18-18: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🧹 Nitpick comments (7)
tests/resources/test_api_keys.py (1)
Line range hint
1-106: Add type hints for better type safetyConsider adding type hints to test functions for better type safety and IDE support.
Example for the first test function:
@pytest.mark.vcr -def test_api_keys_create(): - api_key = ApiKey.create() +def test_api_keys_create() -> None: + api_key: ApiKey = ApiKey.create()🧰 Tools
🪛 Gitleaks (8.21.2)
18-18: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
cuenca/resources/sessions.py (2)
15-17: Good security practice using SecretStr for sensitive data!Using SecretStr for the id field helps prevent accidental exposure of sensitive data in logs.
However, the
# type: ignorecomment should be documented with a reason.- id: SecretStr = None # type: ignore + id: SecretStr = None # type: ignore # None is temporarily needed for Pydantic model initialization
53-54: Document the reason for type ignore comments.The type: ignore comments should include explanations for maintainability.
- success_url=success_url, # type: ignore - failure_url=failure_url, # type: ignore + success_url=success_url, # type: ignore # SessionRequest expects AnyUrlString but str is valid here + failure_url=failure_url, # type: ignore # SessionRequest expects AnyUrlString but str is valid herecuenca/resources/jwt.py (1)
8-12: Add documentation and consider adding model_config.The class implementation looks good, but could benefit from:
- Class-level docstring explaining the purpose and usage
- Field-level documentation for
tokenandapi_key- Consider adding
model_configwith json_schema_extra for consistency with other resources like LoginTokenclass Jwt(Creatable): + """ + Represents a JWT resource for authentication. + + This class handles JWT token creation and management. + """ _resource: ClassVar = 'jwt' - token: SecretStr - api_key: str + token: SecretStr # JWT token stored securely + api_key: str # Associated API key + + model_config = ConfigDict( + json_schema_extra={'example': {'token': '***', 'api_key': 'ak_123'}} + )cuenca/resources/login_tokens.py (2)
9-9: Consider improving type handling instead of using type ignores.The type ignore comments suggest potential type system issues that could be addressed more robustly:
- The class-level mypy disable could be more specific
- The id field type ignore could be resolved by proper type overriding
Try this approach instead:
-# mypy: disable-error-code=override class LoginToken(Creatable): _resource: ClassVar = 'login_tokens' # Override the `id` field to be a `SecretStr` # To ensure sensitive data is not exposed in logs. - id: SecretStr # type: ignore + id: SecretStr = Field(description="Secure token ID")Also applies to: 15-15
Line range hint
21-29: Enhance method documentation with return type info.The create method's docstring could be more descriptive about the return value structure.
@classmethod def create(cls, session: Session = global_session) -> 'LoginToken': """ Use this method to create a token that will last for 7 days Make sure to store this token in a safe place - :return: Token that you can use in cuenca.configure + :return: LoginToken instance with a secure ID that can be used in cuenca.configure + The ID is stored as SecretStr for security. """ return cast('LoginToken', cls._create(session=session))cuenca/resources/otps.py (1)
Line range hint
22-28: Consider adding type hints for session parameter.While not directly related to the Pydantic v2 migration, consider adding type hints to the
sessionparameter in thecreatemethod for better type safety.- def create(cls, session: Session = global_session) -> 'Otp': + def create(cls, *, session: Session = global_session) -> 'Otp':
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
cuenca/resources/__init__.py(2 hunks)cuenca/resources/api_keys.py(4 hunks)cuenca/resources/jwt.py(1 hunks)cuenca/resources/login_tokens.py(1 hunks)cuenca/resources/otps.py(1 hunks)cuenca/resources/sessions.py(3 hunks)cuenca/version.py(1 hunks)requirements.txt(1 hunks)tests/resources/test_api_keys.py(1 hunks)tests/resources/test_login_tokens.py(1 hunks)tests/resources/test_otps.py(1 hunks)tests/resources/test_sessions.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/resources/test_otps.py
- tests/resources/test_sessions.py
- cuenca/resources/api_keys.py
- cuenca/version.py
- requirements.txt
🔇 Additional comments (10)
tests/resources/test_api_keys.py (1)
21-21: LGTM! Correctly updated to use SecretStr's get_secret_value()The change properly adapts to Pydantic v2's SecretStr type for secure handling of sensitive data.
cuenca/resources/sessions.py (4)
4-5: LGTM! Import changes align with Pydantic v2 migration.The updated imports correctly reflect the migration to Pydantic v2 and the new type system.
22-24: LGTM! Type updates align with Pydantic v2.The field types have been correctly updated from AnyUrl to AnyUrlString.
Line range hint
25-38: LGTM! Configuration updated to Pydantic v2 style.The migration from Config class to model_config using ConfigDict follows the recommended Pydantic v2 patterns.
56-57: LGTM! Correctly using model_dump() for Pydantic v2.The migration from dict() to model_dump() aligns with Pydantic v2 best practices.
cuenca/resources/login_tokens.py (2)
17-19: LGTM! Correct usage of Pydantic v2 ConfigDict.The migration from Config class to model_config using ConfigDict follows the Pydantic v2 patterns correctly.
1-1: Verify Python version constraints in project configuration.Since this PR drops Python 3.8 support, ensure that the minimum Python version is properly set in either setup.py or pyproject.toml.
✅ Verification successful
Python version constraints are properly configured ✅
The
setup.pyfile correctly specifiespython_requires='>=3.9', which aligns with dropping Python 3.8 support.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check Python version constraints in project configuration echo "Checking Python version constraints..." find . -maxdepth 1 -type f \( -name "setup.py" -o -name "pyproject.toml" \) -exec grep -H "python" {} \;Length of output: 297
cuenca/resources/otps.py (3)
3-3: LGTM! Proper Pydantic v2 imports.The imports have been correctly updated to use Pydantic v2's new location for
ConfigDictandSecretStr.
11-11: Security improvement: Enhanced secret field protection.Good change! Using
SecretStrinstead ofstrfor thesecretfield provides better security by:
- Preventing accidental exposure in logs and str() representations
- Ensuring the secret is only accessible via the
get_secret_value()methodLet's verify if there are any direct string comparisons with this field that need updating:
✅ Verification successful
Verification complete: SecretStr implementation is consistent throughout the codebase
The codebase already properly handles SecretStr values by using the
get_secret_value()method. No direct string comparisons were found that would need updating.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for potential direct comparisons with the secret field # that might break with the SecretStr change rg -l "secret.*==" --type pyLength of output: 63
Script:
#!/bin/bash # Show the context of the comparison in the test file echo "=== API Keys test comparison context ===" rg "secret.*==" -B 2 -A 2 tests/resources/test_api_keys.py # Search for OTP-specific secret usage echo -e "\n=== OTP secret usage patterns ===" rg "otp.*secret|secret.*otp" --type py -B 2 -A 2Length of output: 607
12-19: Breaking change: Updated Pydantic v2 configuration style.The configuration has been correctly migrated to Pydantic v2 style:
- Replaced
Configclass withmodel_config- Updated
schema_extratojson_schema_extraHowever, this is a breaking change for any code that might be accessing the configuration directly.
Let's verify if there are any other files still using the old configuration style:
| @@ -20,5 +20,5 @@ def test_login_token(session): | |||
| UserLogin.create('222222', session=session) | |||
| login_token = LoginToken.create(session=session) | |||
| session.headers.pop('X-Cuenca-LoginId') | |||
| session.configure(login_token=login_token.id) | |||
| session.configure(login_token=login_token.id.get_secret_value()) | |||
There was a problem hiding this comment.
💡 Codebase verification
Update login token ID access in test_otps.py to use get_secret_value()
The following location needs to be updated to match Pydantic v2's SecretStr pattern:
tests/resources/test_otps.py: Replacelogin_token.idwithlogin_token.id.get_secret_value()
🔗 Analysis chain
LGTM! Correctly implements Pydantic v2's SecretStr pattern.
The change from direct ID access to using get_secret_value() aligns with Pydantic v2's enhanced security features for handling sensitive data.
Let's verify that all direct accesses to login token IDs have been updated across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any remaining direct accesses to login token IDs that haven't been updated to use get_secret_value()
# Look for potential instances where LoginToken.id is accessed directly
rg -l "LoginToken" | xargs rg "\.id[^.]" -A 2 -B 2
# Look for configure calls that might be passing raw login tokens
rg "configure.*login_token.*id[^.]" -A 2 -B 2
Length of output: 1267
…d CardValidation classes
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
setup.py (1)
31-35:⚠️ Potential issueRemove Python 3.13 classifier.
Since Python 3.13 is still in development and not officially released, including it in the classifiers is premature and could mislead users about compatibility.
"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",
🧹 Nitpick comments (3)
cuenca/resources/curp_validations.py (1)
42-44: Required fields should document their mandatory natureFor required fields (calculated_curp, renapo_curp_match, renapo_full_match), consider updating descriptions to indicate they are mandatory.
calculated_curp: CurpField = Field( - description='Calculated CURP based on request data' + description='[Required] Calculated CURP based on request data' ) # ... similar changes for other required fieldsAlso applies to: 48-50, 51-54
cuenca/resources/users.py (2)
33-70: Field definitions updated for Pydantic v2 compatibility.The field definitions have been enhanced with proper type hints and descriptions, which is great for documentation. However, there are some potential improvements:
- Consider adding validation constraints for critical fields
- The optional fields could benefit from more specific default values instead of just None
Example improvement for phone_number field:
- phone_number: Optional[PhoneNumber] = None + phone_number: Optional[PhoneNumber] = Field( + None, + description='User phone number in E.164 format', + examples=['+525511223344'] + )
Line range hint
1-174: Consider documenting Pydantic v2 migration steps.The migration to Pydantic v2 introduces several breaking changes. Consider adding a migration guide in the documentation to help users upgrade their code, including:
- Changes from dict() to model_dump()
- New field definitions with Field
- Config class to model_config changes
- New type hints format (List -> list)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.github/workflows/release.yml(1 hunks).github/workflows/test.yml(3 hunks)cuenca/resources/api_keys.py(4 hunks)cuenca/resources/arpc.py(3 hunks)cuenca/resources/card_validations.py(3 hunks)cuenca/resources/curp_validations.py(4 hunks)cuenca/resources/endpoints.py(5 hunks)cuenca/resources/platforms.py(4 hunks)cuenca/resources/sessions.py(3 hunks)cuenca/resources/users.py(9 hunks)cuenca/resources/verifications.py(4 hunks)cuenca/resources/webhooks.py(1 hunks)cuenca/version.py(1 hunks)requirements.txt(1 hunks)setup.py(1 hunks)tests/resources/cassettes/test_user_beneficiaries_update.yaml(1 hunks)tests/resources/test_cards.py(0 hunks)tests/resources/test_endpoints.py(1 hunks)tests/resources/test_sessions.py(2 hunks)
💤 Files with no reviewable changes (1)
- tests/resources/test_cards.py
🚧 Files skipped from review as they are similar to previous changes (9)
- tests/resources/test_endpoints.py
- cuenca/version.py
- requirements.txt
- cuenca/resources/webhooks.py
- .github/workflows/test.yml
- tests/resources/test_sessions.py
- cuenca/resources/verifications.py
- cuenca/resources/arpc.py
- cuenca/resources/endpoints.py
🔇 Additional comments (21)
.github/workflows/release.yml (1)
9-11: LGTM! Good job updating the GitHub Actions.The update to the latest versions of actions/checkout@v4 and actions/setup-python@v5 is a good security practice.
setup.py (2)
24-24: LGTM! Good job updating the minimum Python version.The update to require Python >=3.9 aligns well with the PR objective to drop Python 3.8 support.
26-28: Verify dependency compatibility with Python versions.Let's verify the compatibility of the dependencies:
✅ Verification successful
Dependencies are compatible with Python requirements
All specified dependencies are compatible with the project's Python >=3.9 requirement:
- requests>=2.32.0 (requires Python >=3.8)
- pydantic-extra-types>=2.10.0 (requires Python >=3.8)
- cuenca-validations>=2.0.0 (no Python version restrictions)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check compatibility of dependencies with Python versions echo "Checking requests package..." curl -s https://pypi.org/pypi/requests/2.32.0/json | jq '.info.requires_python' echo "Checking pydantic-extra-types package..." curl -s https://pypi.org/pypi/pydantic-extra-types/2.10.0/json | jq '.info.requires_python' echo "Checking cuenca-validations package..." curl -s https://pypi.org/pypi/cuenca-validations/2.0.0/json | jq '.info.requires_python'Length of output: 526
cuenca/resources/card_validations.py (5)
6-6: ImportPaymentCardNumberfor enhanced type validationImporting
PaymentCardNumberfrompydantic_extra_types.paymentimproves the specificity and validation of card number inputs, increasing data integrity.
22-26: Initialize optional boolean attributes withNoneSetting default values of
Nonefor optional boolean attributes clarifies their initialization state and prevents potentialAttributeErrors when attributes are accessed before being explicitly set.
34-34: Updatenumberparameter type toPaymentCardNumberChanging the type of the
numberparameter fromstrtoPaymentCardNumberenhances input validation and ensures that only valid card numbers are accepted. This change may affect existing code where astris passed.Please ensure that all calls to
CardValidation.createare updated to provide aPaymentCardNumberinstance instead of astr. If necessary, wrap the card number strings usingPaymentCardNumber(number).
41-41: Changepin_attempts_exceededtype fromOptional[str]toOptional[bool]Updating the
pin_attempts_exceededparameter toOptional[bool]improves type safety and clarity. However, this modification may impact code that passesstrvalues, expecting string representations of booleans.Verify that all invocations of
CardValidation.createprovidepin_attempts_exceededas aboolorNone, and update any code that may be passingstrvalues accordingly.
56-56: Usemodel_dump()instead ofdict()for serializationSwitching to
req.model_dump()aligns with Pydantic v2 conventions for model serialization, ensuring compatibility and leveraging the improved features of the new version.cuenca/resources/sessions.py (2)
53-54: Avoid suppressing type checks with# type: ignoreUsing
# type: ignoreforsuccess_urlandfailure_urlsuppresses valuable type checking that can catch errors. It's better to resolve the type mismatch to ensure robustness.This issue was previously highlighted in past reviews.
Apply the suggested changes to the method parameters to eliminate the need for
# type: ignore.
56-57: Usemodel_dump()instead ofdict()for serializationUpdating to
req.model_dump()aligns with Pydantic v2's serialization practices, ensuring compatibility and taking advantage of the latest features.cuenca/resources/api_keys.py (4)
5-5: ImportConfigDictfor Pydantic v2 configurationUsing
ConfigDictfrom Pydantic simplifies model configuration and aligns with the updated practices in Pydantic v2.
16-17: Add defaultNonevalues to optional fieldsInitializing
deactivated_atanduser_idwithNoneexplicitly marks these fields as optional, enhancing code clarity and preventing potentialAttributeErrors.
Line range hint
18-29: ReplaceConfigclass withmodel_configTransitioning from the
Configclass tomodel_configwithConfigDictis consistent with Pydantic v2 best practices, streamlining model configuration and schema generation.
78-78: Usemodel_dump()instead ofdict()inupdatemethodSwitching to
req.model_dump()ensures compatibility with Pydantic v2's serialization methods, maintaining consistency across the codebase.cuenca/resources/platforms.py (3)
15-34: Field definitions follow Pydantic v2 best practicesThe migration to using
Fieldwith descriptions improves schema documentation and type validation. The implementation correctly follows Pydantic v2 patterns.
Line range hint
35-50: Verify schema example date formatThe schema example uses an ISO datetime format for
establishment_date('2021-08-24T14:15:22Z') but the field type isdt.date. Consider updating the example to use date-only format ('2021-08-24').
76-78: Correctly migrated from dict() to model_dump()The change from
dict()tomodel_dump()aligns with Pydantic v2's API changes.tests/resources/cassettes/test_user_beneficiaries_update.yaml (1)
32-32: Verify test coverage for API response changes.The cassette shows changes in the API response format:
- Phone number format changed to "+529988776666"
- New
created_attimestamps added to beneficiariesRun this script to check if all related tests are updated to handle these changes:
✅ Verification successful
No test updates needed for API response changes
The test implementation focuses on sending beneficiary data and doesn't make assertions about the response format. The changes in phone number format and addition of
created_attimestamps are passive response data that don't affect test behavior.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for tests that might need updates due to API response changes # Search for tests using phone numbers or beneficiary fields rg -l "529988776" tests/ rg -l "created_at.*beneficiaries" tests/ # Look for other test cassettes that might need similar updates fd -e yaml . tests/resources/cassettes/ --exec grep -l "phone_number\|beneficiaries"Length of output: 1521
Script:
#!/bin/bash # Check test implementations for phone number format or beneficiary structure assumptions ast-grep --pattern 'def test_user_beneficiaries$$$' # Also check for any assertions on phone numbers or beneficiary fields rg -A 3 "assert.*phone_number|assert.*beneficiar" tests/Length of output: 112
Script:
#!/bin/bash # Search for test implementations with broader patterns rg -A 5 "test.*beneficiar" tests/ rg -A 5 "assert" tests/test_user_beneficiaries_update.py || true # Look for the actual test file fd "test_user_beneficiaries" tests/ --exec cat {}Length of output: 3455
cuenca/resources/users.py (3)
Line range hint
77-101: Model configuration updated to Pydantic v2 style.Good migration from Config class to model_config using ConfigDict. The example schema is comprehensive and helpful for API documentation.
Line range hint
136-174: Update method signature changed and modernized.The method has been updated to use model_dump() and includes new parameter curp_document_uri. This is a breaking change that requires attention.
Let's verify the impact:
✅ Verification successful
Optional parameter addition and internal changes, no breaking impact
The changes are backward compatible:
- New parameter
curp_document_uriis optional with None as default- Existing calls in test files remain compatible
- No current usage of curp_document feature found
- Internal model_dump() change doesn't affect callers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for existing update() calls that might need modification # Search for update method calls that might be affected rg "User\.update\(" --type py # Look for any references to the old parameter name rg "curp_document[^_]" --type pyLength of output: 403
Line range hint
104-133: Create method updated for Pydantic v2 compatibility.The method has been properly updated to use model_dump() instead of dict(). The addition of optional id parameter is a breaking change that should be documented.
Let's check for any direct instantiations that might be affected:
| id: str | ||
| # Override the `id` field to be a `SecretStr` | ||
| # To ensure sensitive data is not exposed in logs. | ||
| id: SecretStr = None # type: ignore |
There was a problem hiding this comment.
Assigning None to SecretStr field id may cause type issues
Assigning None to id: SecretStr leads to a type mismatch since SecretStr expects a string value. Using # type: ignore suppresses the type checker but does not address the underlying issue.
Apply this diff to fix the issue:
- id: SecretStr = None # type: ignore
+ id: Optional[SecretStr] = NoneThis change explicitly allows id to be None, resolving the type mismatch without suppressing type checks.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| id: SecretStr = None # type: ignore | |
| id: Optional[SecretStr] = None |
| @@ -94,11 +94,11 @@ def create( | |||
| second_surname=second_surname, | |||
| date_of_birth=date_of_birth, | |||
| state_of_birth=state_of_birth, | |||
| country_of_birth=country_of_birth, | |||
| country_of_birth=cast(Country, country_of_birth), | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using TypeAdapter for safer type casting
Instead of using cast(), consider using Pydantic's TypeAdapter for safer runtime type conversion of country_of_birth.
+ from pydantic import TypeAdapter
# ...
- country_of_birth=cast(Country, country_of_birth),
+ country_of_birth=TypeAdapter(Country).validate_python(country_of_birth) if country_of_birth else None,Also applies to: 103-103
| date_of_birth: Optional[dt.date] = Field( | ||
| None, description='In format ISO 3166 Alpha-2' | ||
| ) |
There was a problem hiding this comment.
Incorrect description for date_of_birth field
The description "In format ISO 3166 Alpha-2" is incorrect for a date field. This appears to be copied from country/nationality fields.
- date_of_birth: Optional[dt.date] = Field(
- None, description='In format ISO 3166 Alpha-2'
- )
+ date_of_birth: Optional[dt.date] = Field(
+ None, description='Date of birth in ISO format (YYYY-MM-DD)'
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| date_of_birth: Optional[dt.date] = Field( | |
| None, description='In format ISO 3166 Alpha-2' | |
| ) | |
| date_of_birth: Optional[dt.date] = Field( | |
| None, description='Date of birth in ISO format (YYYY-MM-DD)' | |
| ) |
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 release notes:
Release Notes for Version 2.0.0
Major Changes
New Features
SecretStrBreaking Changes
.dict()to.model_dump()ListtolistDependency Updates
requeststo 2.32.3cuenca-validationsto 2.0.0pydantic-extra-typesdataclassesrequirementImprovements