Dependency/update cuenca validations #413
Conversation
WalkthroughThis pull request introduces several modifications across multiple files in the Cuenca project. The primary changes involve updating URL-related type annotations from standard Pydantic types like Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #413 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 49 49
Lines 1087 1095 +8
=========================================
+ Hits 1087 1095 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
tests/resources/test_user_credentials.py (1)
10-12: Add test cases for password validation.While the password length updates align with the new minimum length requirement, consider adding test cases for:
- Passwords shorter than 8 characters (should fail)
- Edge cases around the 8-character limit
- Password validation error scenarios
cuenca/resources/user_credentials.py (2)
29-32: Consider updating type hints for password parameter.Since the password field in UserCredentialRequest is now SecretStr, consider updating the type hint of the password parameter from
strtostr | SecretStrto make it more explicit.
47-50: Verify SecretStr handling in update method.The implementation correctly handles password serialization, but consider:
- Updating the password parameter type hint to
Optional[str | SecretStr]- Adding a comment explaining why we need to handle SecretStr specifically
setup.py (1)
27-27: Consider adding an upper bound to the version constraint.While
>=2.0.4allows using the new serializable URL types, it's recommended to specify an upper bound (e.g.,>=2.0.4,<3.0.0) to prevent potential breaking changes from future major versions.- 'cuenca-validations>=2.0.4', + 'cuenca-validations>=2.0.4,<3.0.0',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
cuenca/resources/endpoints.py(4 hunks)cuenca/resources/files.py(2 hunks)cuenca/resources/sessions.py(2 hunks)cuenca/resources/user_credentials.py(3 hunks)cuenca/resources/users.py(2 hunks)cuenca/version.py(1 hunks)requirements.txt(1 hunks)setup.py(1 hunks)tests/resources/cassettes/test_update_password.yaml(3 hunks)tests/resources/test_user_credentials.py(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- requirements.txt
- cuenca/version.py
🔇 Additional comments (8)
cuenca/resources/user_credentials.py (1)
8-8: LGTM! Import of SecretStr from pydantic.The import aligns with the PR objective of adapting to the updated password field type.
tests/resources/cassettes/test_update_password.yaml (1)
3-3: LGTM! Consistent password length updates in test fixtures.The password length updates in the test fixtures align with the new minimum length requirement and maintain consistency with the test file changes.
Also applies to: 57-57, 111-111
cuenca/resources/sessions.py (1)
20-21: LGTM! Type updates are consistent with PR objectives.The change from
Optional[AnyUrl]toOptional[SerializableAnyUrl]aligns with the goal of improving URL serialization.cuenca/resources/files.py (1)
17-17: LGTM! Type update is consistent with PR objectives.The change from
HttpUrltoSerializableHttpUrlaligns with the goal of improving URL serialization.cuenca/resources/endpoints.py (1)
18-18: LGTM! Type updates are comprehensive and consistent.The changes from
HttpUrltoSerializableHttpUrlare consistently applied across:
- Class field definition
- create method parameter
- update method parameter
This aligns with the goal of improving URL serialization.
Also applies to: 55-55, 76-76
cuenca/resources/users.py (3)
18-18: LGTM! Import changes align with the migration to serializable URL types.The addition of
SerializableHttpUrlfromcuenca_validations.types.generalis consistent with the PR's objective to update URL-related type annotations.
Line range hint
155-169: Verify UserUpdateRequest compatibility with SerializableHttpUrl.The usage of
curp_documentin the request model looks good. Let's verify thatUserUpdateRequestin cuenca-validations is updated to handleSerializableHttpUrl.
151-151: Verify consistent usage of SerializableHttpUrl for CURP documents.The type update for
curp_documentlooks good. Let's verify that this change is consistently applied across the codebase.✅ Verification successful
Type change for curp_document is consistently applied
The codebase shows consistent usage of
Optional[SerializableHttpUrl]for thecurp_documentparameter, with no remaining instances of the old type.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining HttpUrl usage for CURP documents rg -l "HttpUrl.*curp_document|curp_document.*HttpUrl" # Search for SerializableHttpUrl usage with CURP documents to ensure consistency rg -l "SerializableHttpUrl.*curp_document|curp_document.*SerializableHttpUrl"Length of output: 187
Script:
#!/bin/bash # Show all occurrences of HttpUrl and SerializableHttpUrl with CURP documents with context rg "HttpUrl.*curp_document|curp_document.*HttpUrl|SerializableHttpUrl.*curp_document|curp_document.*SerializableHttpUrl" -C 3Length of output: 592
Description
This PR updates the project to use the latest version of
cuenca-validationsChanges
HttpUrlandAnyUrlwithSerializableHttpUrlandSerializableAnyUrl, respectively, to avoid serialization issues.passwordfield incuenca-validations, which is now of typeSecretStrwith a minimum of 8 characters.user_credentials.pyto handle serialization ofpasswordwhen it is aSecretStrby using itsget_secret_value()method.Summary by CodeRabbit
Release Notes v2.0.1
New Features
Bug Fixes
Chores
cuenca-validationspackage to version 2.0.4Internal Improvements