Update/py3 13 deps #148
Update/py3 13 deps #148
Conversation
WalkthroughThis pull request updates numerous aspects of the project. The GitHub Actions workflows have been modified to use newer action versions and to upgrade the Python version from 3.8 to 3.13. The Makefile reflects this Python version change and updates the formatting target for Black. The README has been rebranded from “cuenca” to “cepmex” and reorganized with new sections for installation, development, testing, usage, and error handling. Core library changes include renaming an attribute in the 📜 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 (
|
![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 (10)
cep/exc.py (1)
15-19: LGTM! Consider adding error details parameter.The new exception class follows the established pattern and is well-documented. Consider adding an optional parameter to capture specific details about which parts of the response were incomplete.
class IncompleteResponseError(CepError): """ Respuesta incompleta del sitio web https://www.banxico.org.mx/cep/ """ + def __init__(self, message: str = None): + super().__init__(message or self.__doc__)setup.py (1)
31-34: Consider adding Python version classifiers gradually.Adding support for Python 3.13 (which is in alpha) might be premature. Consider adding it later when it's more stable.
classifiers=[ 'Programming Language :: Python :: 3.10', 'Programming Language :: Python :: 3.11', 'Programming Language :: Python :: 3.12', - 'Programming Language :: Python :: 3.13', 'License :: OSI Approved :: MIT License', 'Operating System :: OS Independent', ],tests/test_transferencia.py (2)
8-9: Consider organizing imports.The new imports could be grouped with related imports for better organization.
import pytest from requests import HTTPError -from cep import Transferencia -from cep.client import Client -from cep.exc import CepError, IncompleteResponseError, MaxRequestError +from cep import Transferencia +from cep.client import Client +from cep.exc import ( + CepError, + IncompleteResponseError, + MaxRequestError, +)
95-120: Enhance mock response to test specific missing fields.The mock XML response could be more specific about which fields are missing to better test the
IncompleteResponseErrorscenario.mock_get_response = ( b'<?xml version="1.0" encoding="UTF-8"?>' - b'<SPEI_Tercero FechaOperacion="2019-04-12" Hora="13:31:44">' - b'</SPEI_Tercero>' + b'<SPEI_Tercero FechaOperacion="2019-04-12" Hora="13:31:44">' + b' <ClaveRastreo>CUENCA927820173168</ClaveRastreo>' + b' <!-- Missing required fields like Ordenante, Beneficiario -->' + b'</SPEI_Tercero>' )cep/transferencia.py (2)
66-68: Consider making the default time value more explicit.While the default value handling is good, consider making the magic value '00:00:00' more explicit by defining it as a constant.
+ DEFAULT_TIME = '00:00:00' ... - hora = resp.get('Hora') or '00:00:00' + hora = resp.get('Hora') or DEFAULT_TIME
88-90: Consider adding a more descriptive error message.While the error handling is improved, consider adding a descriptive message to the
CepErrorto help with debugging.- raise CepError + raise CepError("Failed to validate client in descargar method")Also applies to: 99-100
requirements-test.txt (1)
1-9: Consider constraining major version ranges for critical testing dependencies.While using version ranges provides flexibility, consider constraining major versions for critical testing dependencies to prevent unexpected breaking changes.
- pytest==8.* + pytest~=8.0 - pytest-cov==6.* + pytest-cov~=6.0 - mypy==1.* + mypy~=1.0.github/workflows/release.yml (1)
10-13: Consider using a matrix strategy for Python versions.Since the PR aims to support Python 3.10-3.13, consider using a matrix strategy to test the release process across all supported versions.
- name: Set up Python 3.13 uses: actions/setup-python@v5 with: - python-version: 3.13 + strategy: + matrix: + python-version: ['3.10', '3.11', '3.12', '3.13'] + with: + python-version: ${{ matrix.python-version }}Makefile (1)
3-3: Consider using Python 3.10 for broader compatibility.Since the project now supports Python 3.10-3.13 (as per PR objectives), consider using Python 3.10 and
--target-version py310for the development environment. This ensures that the code formatting aligns with the lowest supported Python version, maximizing compatibility across all supported versions.-PYTHON=python3.11 +PYTHON=python3.10 -black = black -S -l 79 --target-version py311 $(PROJECT) tests setup.py +black = black -S -l 79 --target-version py310 $(PROJECT) tests setup.pyAlso applies to: 6-6
.github/workflows/test.yml (1)
38-42: Consider using the lowest supported Python version for coverage.For coverage reporting, consider using Python 3.10 instead of 3.13 to ensure we're capturing any potential syntax or feature compatibility issues with the lowest supported version.
- python-version: 3.13 + python-version: '3.10'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.github/workflows/release.yml(1 hunks).github/workflows/test.yml(3 hunks)Makefile(1 hunks)README.md(1 hunks)cep/cuenta.py(1 hunks)cep/exc.py(1 hunks)cep/transferencia.py(4 hunks)requirements-test.txt(1 hunks)requirements.txt(1 hunks)setup.py(1 hunks)tests/test_transferencia.py(2 hunks)
🔇 Additional comments (11)
cep/cuenta.py (1)
2-2: Verify impact of making fields optional.Making all fields optional is a breaking change that could affect existing code. This change aligns with modern Python typing but needs careful consideration.
Let's check for direct usage of these fields:
Also applies to: 9-13
✅ Verification successful
✓ Optional fields are safely handled in dependent code
The change to make fields optional is safe. The only dependent file (transferencia.py) already implements proper null checks and defensive programming patterns when accessing these fields.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for direct attribute access that might break rg -l "\.nombre\b|\.tipo\b|\.banco\b|\.numero\b|\.rfc\b" --type pyLength of output: 89
Script:
#!/bin/bash # Check how these fields are used in transferencia.py rg -A 3 -B 3 "\.nombre\b|\.tipo\b|\.banco\b|\.numero\b|\.rfc\b" cep/transferencia.pyLength of output: 366
setup.py (1)
24-24: Verify compatibility with major dependency updates.The update includes significant version bumps:
- clabe 1.x → 2.x (includes Pydantic 2.0 changes)
- lxml 4.x → 5.x
- requests 2.25 → 2.32
These changes require careful testing, especially the clabe update which might introduce breaking changes.
Let's check for potential compatibility issues:
Also applies to: 26-28
✅ Verification successful
Dependencies update is safe to proceed ✅
The codebase uses only stable core features from the updated dependencies:
- clabe: Only the
BANKSconstant for validation- lxml: Basic
etreefunctionality- requests: Standard
HTTPErrorexceptionNo deprecated features or complex APIs are used, making these version updates safe.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if we're using any deprecated features from these libraries rg -A 3 "from (requests|clabe|lxml|pydantic) import" --type py # Check if requirements-dev.txt is in sync if [ -f "requirements-dev.txt" ]; then echo "Checking requirements-dev.txt..." cat requirements-dev.txt fiLength of output: 614
Script:
#!/bin/bash # Check for any clabe usage patterns rg -i "clabe" --type py -A 3 # Check for pydantic models or validation ast-grep --pattern 'class $_ (BaseModel|$_Model)'Length of output: 704
tests/test_transferencia.py (1)
123-130: LGTM! Good coverage of validation failure.The test properly covers the scenario where validation fails and ensures the appropriate error is raised.
cep/transferencia.py (2)
11-11: LGTM: New exception import aligns with error handling improvements.The addition of
IncompleteResponseErrorimport supports the enhanced XML response validation.
57-64: LGTM: Robust validation of XML response elements.The changes properly validate the presence of required XML elements before accessing them, preventing potential
NoneTypeerrors.requirements.txt (1)
1-3: Verify package compatibility with Python 3.10-3.13.Let's verify the Python version compatibility of the updated packages.
✅ Verification successful
✓ All packages are compatible with Python 3.10-3.13
- requests 2.32.3 (requires Python >=3.8)
- clabe 2.0.0 (requires Python >=3.8)
- lxml 5.3.0 (requires Python >=3.6)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check Python version compatibility for updated packages echo "Checking Python version compatibility..." # Check PyPI for package metadata for package in requests clabe lxml; do echo "=== $package ===" curl -s "https://pypi.org/pypi/$package/json" | \ jq -r '.info | "Latest version: " + .version + "\nPython requires: " + .requires_python' doneLength of output: 856
.github/workflows/release.yml (1)
20-20: LGTM: Using stable release version for PyPI publish action.Using a stable release version (
release/v1) instead ofmasterimproves reliability.README.md (1)
5-5: Verify repository links after package rename.While the package has been renamed from
cuencatocepmex, the GitHub repository links in the badges still referencecuenca-mx. Consider updating these for consistency if the repository will also be renamed.✅ Verification successful
Repository links are correct, no changes needed
The package rename from
cuencatocepmexon PyPI is independent of the repository location. The current GitHub repository links correctly point tocuenca-mx/cep-python, which remains the valid and active location of the project.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the repository name has changed or if there are plans to rename it gh api repos/cuenca-mx/cep-pythonLength of output: 6769
.github/workflows/test.yml (3)
9-13: LGTM! GitHub Actions versions are up-to-date.The upgrade to the latest stable versions of actions is a good practice:
- checkout@v4
- setup-python@v5
23-23: LGTM! Python versions matrix aligns with objectives.The test matrix correctly includes all supported Python versions (3.10-3.13) as specified in the PR objectives.
48-50: LGTM! Codecov integration is properly secured.Good addition of the Codecov token for secure upload of coverage reports.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #148 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 6 6
Lines 98 128 +30
=========================================
+ Hits 98 128 +30
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (16)
cep/transferencia.py (1)
75-77: Consider using a more robust date parsing approach.The current date extraction from
cadenaCDAstring uses string slicing which can be fragile. Consider using a more robust approach with proper error handling.- fecha_abono_str = ( - f"{cadena_cda[4][4:]}-{cadena_cda[4][2:4]}-{cadena_cda[4][:2]}" - ) + try: + fecha_str = cadena_cda[4] + fecha_abono_str = f"{fecha_str[4:]}-{fecha_str[2:4]}-{fecha_str[:2]}" + datetime.datetime.strptime(fecha_abono_str, '%Y-%m-%d') # Validate format + except (IndexError, ValueError) as e: + raise CepError(f"Invalid date format in cadenaCDA: {e}") from etests/test_transferencia.py (1)
38-39: Replace unused loop variable with underscore.The loop control variable
iis not used within the loop body.- for i in range(10): + for _ in range(10):🧰 Tools
🪛 Ruff (0.8.2)
38-38: Loop control variable
inot used within loop bodyRename unused
ito_i(B007)
tests/cassettes/test_fail_validar_transferencia.yaml (1)
15-15: Verify beta environment configuration.The test fixtures are using the
cep-betaendpoint. Ensure this aligns with the intended testing strategy and that there's a plan to update these when moving to production.Also applies to: 19-19, 44-44, 52-52
tests/cassettes/test_maximo_numero_de_requests.yaml (1)
3-3: Consider using relative dates in test data.The test uses a fixed future date
08-11-2024which could cause the test to fail when that date is reached. Consider using a dynamic date relative to the current date to ensure the test remains valid.- body: tipoCriterio=T&captcha=c&tipoConsulta=1&fecha=08-11-2024&criterio=MIFELSPEI20241108102122835&emisor=40042&receptor=90723&cuenta=723969000011000077&monto=20912.98&receptorParticipante=0 + body: !helper_function 'generate_request_body_with_relative_date(days_from_now=30)'tests/cassettes/test_validar_transferencia_tipo_4.yaml (1)
19-19: Consider enforcing HTTPS for all requests.The initial request uses HTTP which then redirects to HTTPS. While the redirect ensures secure communication, it's better to use HTTPS directly to prevent potential security risks like man-in-the-middle attacks during the initial request.
Update the base URL to use HTTPS:
- uri: http://www.banxico.org.mx/cep-beta/valida.do + uri: https://www.banxico.org.mx/cep-beta/valida.dotests/cassettes/test_validar_transferencia_tipo_3.yaml (7)
19-19: Verify HTTPS usage in test environment.The test cassette shows HTTP to HTTPS redirection. While this works, consider updating the initial request to use HTTPS directly to avoid unnecessary redirects and align with security best practices.
- uri: http://www.banxico.org.mx/cep-beta/valida.do + uri: https://www.banxico.org.mx/cep-beta/valida.doAlso applies to: 52-52
149-158: Consider masking sensitive data in test fixtures.The XML response contains sensitive information like account numbers, RFCs (tax IDs), and personal names. Consider using consistent test data with clearly marked dummy values.
For example:
- Use
TEST-RFC-XXXformat for RFCs- Use
TEST-ACCOUNT-XXXfor account numbers- Use
TEST-NAME-XXXfor personal names
151-153: Consider documenting type-specific fields.Type 30 transfer includes additional fields in the XML response. Consider adding documentation or comments explaining the significance of these type-specific fields for better test maintainability.
153-155: Document international transfer fields.Type 35 transfer includes international-specific fields (
US,USD) and additional party information. Consider adding documentation explaining:
- Required fields for international transfers
- Valid currency and country code combinations
- Validation rules for international transfers
3-3: Document tracking ID format requirements.Type 12 transfer uses a different
claveRastreoformat (EPRU723PRENOM24110744VL0000001). Consider documenting:
- Format requirements for each transfer type
- Validation rules for tracking IDs
- Meaning of each segment in the ID
153-155: Document multi-party transaction requirements.Type 36 transfer includes additional party information and intermediary details. Consider documenting:
- Required fields for multi-party transactions
- Validation rules for intermediary information
- Relationship between different parties in the transaction
1-183: Consider enhancing test data management and documentation.The test cassettes for different transfer types share common patterns but lack comprehensive documentation. Consider implementing:
A centralized test data management system to:
- Maintain consistent dummy data across all tests
- Mask sensitive information
- Document valid test data patterns
A test documentation guide explaining:
- Required fields for each transfer type
- Validation rules and constraints
- Expected behavior and edge cases
This will improve test maintainability and make it easier for new contributors to understand the test scenarios.
tests/cassettes/test_lanza_cep_error_para_errores_500.yaml (2)
19-19: Environment URL changes need to be documented.The service endpoints have been updated to use the beta environment (
cep-beta). This change should be documented in the PR description or comments to explain why beta is being used and any implications.Also applies to: 52-52, 197-197, 230-230, 375-375, 408-408, 553-553, 586-586, 731-731, 764-764
149-158: Consider masking sensitive data in test responses.The XML responses contain potentially sensitive information like RFC numbers, account numbers, and personal names. Consider masking this data in test fixtures.
Apply this diff to mask sensitive data:
- RFC=\"LOHF890619HCSPRL05\" Concepto=\"CONCEPTO PAGO TIPO 30\" IVA=\"0.00\" + RFC=\"XXXX890619XXXXXX\" Concepto=\"CONCEPTO PAGO TIPO 30\" IVA=\"0.00\" - Cuenta=\"723969000011000077\" + Cuenta=\"XXXXXXXXXXXX0077\" - Nombre=\"Felipe Lopez Hernandez\" + Nombre=\"XXXX XXXX XXXX\"Also applies to: 327-336, 505-514, 683-692
tests/cassettes/test_validar_transferencia_tipo_30.yaml (1)
3-3: Consider using environment variables for sensitive test data.The request body contains potentially sensitive information like account numbers and transaction amounts. While this is test data, it's a good practice to use environment variables or test constants for such values.
Example using environment variables:
- body: tipoCriterio=T&captcha=c&tipoConsulta=1&fecha=08-11-2024&criterio=BiB2024110810162420780&emisor=37166&receptor=90723&cuenta=723969000011000077&monto=25208.26&receptorParticipante=0 + body: tipoCriterio=T&captcha=c&tipoConsulta=1&fecha=${TEST_DATE}&criterio=${TEST_CRITERIO}&emisor=${TEST_EMISOR}&receptor=${TEST_RECEPTOR}&cuenta=${TEST_CUENTA}&monto=${TEST_MONTO}&receptorParticipante=0Also applies to: 36-36
tests/cassettes/test_validar_transferencia_tipo_36.yaml (1)
1-183: Add test cases for additional error scenarios.The test suite covers successful transfer validations for types 12, 30, 35, and 36, along with basic HTTP errors. Consider adding test cases for:
- Invalid credentials
- Network timeouts
- Malformed responses
- Rate limiting
Would you like me to help generate additional test cases for these scenarios?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
CEP-20241108-BiB202411081016248360.pdfis excluded by!**/*.pdftests/CEP-20190412-CUENCA1555093850.pdfis excluded by!**/*.pdftests/CEP-20241108-BiB202411081016248360.pdfis excluded by!**/*.pdf
📒 Files selected for processing (29)
cep/__init__.py(1 hunks)cep/client.py(2 hunks)cep/config.py(1 hunks)cep/cuenta.py(1 hunks)cep/exc.py(1 hunks)cep/transferencia.py(6 hunks)cep/version.py(1 hunks)tests/cassettes/test_descagar_transferencia_con_fecha_distinta.yaml(0 hunks)tests/cassettes/test_fail_validar_transferencia.yaml(2 hunks)tests/cassettes/test_http_error.yaml(3 hunks)tests/cassettes/test_lanza_cep_error_para_errores_500.yaml(37 hunks)tests/cassettes/test_maximo_numero_de_requests.yaml(8 hunks)tests/cassettes/test_validar_transferencia.yaml(0 hunks)tests/cassettes/test_validar_transferencia_tipo_1.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_10.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_11.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_12.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_3.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_30.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_31.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_35.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_36.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_4.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_5.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_6.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_8.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_9.yaml(1 hunks)tests/conftest.py(2 hunks)tests/test_transferencia.py(1 hunks)
💤 Files with no reviewable changes (2)
- tests/cassettes/test_validar_transferencia.yaml
- tests/cassettes/test_descagar_transferencia_con_fecha_distinta.yaml
✅ Files skipped from review due to trivial changes (1)
- cep/config.py
🚧 Files skipped from review as they are similar to previous changes (2)
- cep/version.py
- cep/exc.py
🧰 Additional context used
🧠 Learnings (1)
cep/cuenta.py (1)
Learnt from: felipao-mx
PR: cuenca-mx/cep-python#148
File: cep/cuenta.py:9-9
Timestamp: 2025-02-07T23:37:20.178Z
Learning: In Python 3.10 and above, use `str | None = None` instead of `Optional[str] = None` for optional string type hints, as it's more readable and uses the newer union type syntax.
🪛 Ruff (0.8.2)
tests/test_transferencia.py
38-38: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
🔇 Additional comments (26)
cep/__init__.py (1)
1-1: LGTM!The changes correctly expose the
Configclass through__all__and add the corresponding import statement.Also applies to: 4-4
cep/cuenta.py (2)
9-9: LGTM!The attribute rename from
tipototipo_cuentais consistent throughout the file.Also applies to: 24-24
9-9: Update type hint to use Python 3.10+ syntax.Since this PR is updating Python compatibility to 3.10+, we can use the newer union type syntax.
- tipo_cuenta: str + tipo_cuenta: str | None = Nonecep/client.py (1)
3-3: LGTM!The changes improve configurability by:
- Moving the base URL to a centralized
Configclass- Correctly using
Config.BASE_URLin therequestmethodAlso applies to: 31-31
tests/conftest.py (2)
7-8: LGTM!Good practice to use a beta endpoint for testing to avoid impacting production.
17-52: LGTM!The fixture has been appropriately renamed and updated with comprehensive test data.
cep/transferencia.py (2)
26-38: LGTM! Improved data type accuracy.The changes to use more precise data types (
Decimalfor monetary values,dateforfecha_operacion) align with best practices for financial systems.
111-124:Details
❓ Verification inconclusive
Verify the impact of using fecha_abono instead of fecha_operacion.
The change from
fecha_operaciontofecha_abono.date()might affect existing code that depends on this behavior.
🏁 Script executed:
#!/bin/bash # Search for any direct usage of fecha_operacion in the codebase rg -A 5 'fecha_operacion'Length of output: 1326
Action Required: Verify Impact of Date Handling Change
While the update to use
self.fecha_abono.date()in thedescargarmethod aligns with the new date handling strategy, the codebase still referencesfecha_operacionin several locations. Please verify that:
- The change does not inadvertently break any existing logic or tests that depend on
fecha_operacionbeing adatetime.dateversus extracting a date fromfecha_abono(adatetime.datetime).- All consumers—such as other modules or test fixtures (e.g., those in
tests/conftest.py)—are updated accordingly to handle the potentially different behavior resulting from this change.tests/test_transferencia.py (1)
65-283: LGTM! Comprehensive test coverage for payment types.The test suite now covers all payment types (1, 3-6, 8-12, 30-31, 35-36) with specific assertions for each type's unique characteristics.
tests/cassettes/test_maximo_numero_de_requests.yaml (1)
19-19:Details
❓ Verification inconclusive
Verify beta endpoint compatibility.
The test is using the beta endpoint
cep-beta. Ensure this aligns with the intended testing environment and verify if there are any differences in behavior between the beta and production endpoints.Also applies to: 52-52
🌐 Web query:
What are the differences between Banxico's CEP beta and production endpoints?💡 Result:
Banco de México's CEP (Electronic Payment Receipt) system provides two distinct endpoint environments for accessing payment validation data:
Key Differences Between CEP Beta and Production Endpoints
1. Data Availability Window
- Production: Allows querying SPEI® payments made within the last 45 working days[3][7]
- Beta: No explicit retention limit mentioned in documentation, suggesting experimental extended access during testing phases[8]
2. Validation Features
- Production:
- Supports XML validation with digital seals using Banco de México's official security certificates[3][4]
- Provides standardized PDF/XML receipts with chain-of-custody tracking[3][8]
- Beta: Likely includes experimental validation methods or enhanced cryptographic features under development[8]
3. API Characteristics
- Production:
- Uses RESTful endpoints with strict parameter requirements (date, reference number, bank IDs)[7][8]
- Enforces 30-second SLA for payment processing confirmation[3][5]
- Beta:
- May feature alternative protocols like SOAP or GraphQL based on GitHub implementation patterns[4][6]
- Could allow relaxed parameter constraints for testing edge cases[8]
4. Operational Stability
- Production: Maintains 99.98% uptime with redundant infrastructure[1][3]
- Beta: Subject to planned maintenance windows and potential instability during feature rollouts[1][8]
5. Access Control
- Production: Requires valid banking credentials through participant institutions[3][7]
- Beta: May use separate authentication mechanisms like API keys for developer testing[4][8]
Implementation Considerations
The Python client library (cuenca-mx/cep-python) primarily interacts with production endpoints but contains patterns adaptable for beta environment testing through configuration overrides[4]. Banco de México's operational manuals indicate beta systems undergo rigorous security audits matching PFMI standards before promotion to production[1][3].
Citations:
- 1: https://www.banxico.org.mx/payment-systems/d/%7B90965A55-8F44-7DD2-45CF-2BF1D7C0B75B%7D.pdf
- 2: https://www.banxico.org.mx/SieAPIRest/service/v1/?locale=en
- 3: https://www.banxico.org.mx/services/interbanking-electronic-payme.html
- 4: https://github.com/cuenca-mx/cep-python
- 5: https://www.banxico.org.mx/services/spei_-transfers-banco-mexico.html
- 6: https://ru.dgb.unam.mx/bitstream/20.500.14330/TES01000827309/3/0827309.pdf
- 7: https://www.banxico.org.mx/services/spei_-information-module-tran.html
- 8: https://docs.kiban.cloud/reference/cep
Action: Confirm Beta Endpoint Compatibility
The cassette in
tests/cassettes/test_maximo_numero_de_requests.yaml(lines 19 and 52) currently uses the beta endpoint:uri: http://www.banxico.org.mx/cep-beta/valida.doPlease verify that this choice is intentional. Based on external documentation, note the following key differences between Banxico's CEP beta and production endpoints:
- Data Availability: Production endpoints support querying payments from the last 45 working days, while the beta endpoint does not specify a retention limit.
- Validation Features: Production uses standardized XML validations with official security certificates, whereas beta may employ experimental validation methods.
- API Characteristics & Stability: The beta environment might offer alternative protocols (e.g., SOAP or GraphQL) and could be less stable compared with production’s high uptime and strict parameter requirements.
- Access Control: Production requires strict authentication, while the beta endpoint may utilize different, possibly more lenient, mechanisms.
Ensure that these differences align with the intended testing environment and that any behavioral variations (if applicable) are correctly handled by your tests.
tests/cassettes/test_validar_transferencia_tipo_31.yaml (1)
1-180: LGTM! Test cassette properly captures type 31 transfer validation.The test data and responses correctly simulate the validation flow for type 31 transfers, including proper handling of redirects and XML response structure.
tests/cassettes/test_validar_transferencia_tipo_6.yaml (1)
1-180: LGTM! Test cassette properly captures type 6 transfer validation.The test data and responses correctly simulate the validation flow for type 6 transfers, including proper handling of redirects and XML response structure.
tests/cassettes/test_validar_transferencia_tipo_10.yaml (1)
1-180: LGTM! Test cassette properly captures type 10 transfer validation.The test data and responses correctly simulate the validation flow for type 10 transfers, including proper handling of redirects and XML response structure.
tests/cassettes/test_validar_transferencia_tipo_5.yaml (1)
1-180: LGTM! Test cassette properly captures type 5 transfer validation.The test data and responses correctly simulate the validation flow for type 5 transfers, including proper handling of redirects and XML response structure.
tests/cassettes/test_validar_transferencia_tipo_11.yaml (1)
1-181: Previous comments apply to this file.The same security concerns about real-looking test data and HTTP usage apply to this file.
tests/cassettes/test_validar_transferencia_tipo_8.yaml (1)
1-181: Previous comments apply to this file.The same security concerns about real-looking test data and HTTP usage apply to this file.
tests/cassettes/test_validar_transferencia_tipo_1.yaml (1)
1-181: Previous comments apply to this file.The same security concerns about real-looking test data and HTTP usage apply to this file.
tests/cassettes/test_validar_transferencia_tipo_9.yaml (1)
1-181: Previous comments apply to this file.The same security concerns about real-looking test data and HTTP usage apply to this file.
tests/cassettes/test_lanza_cep_error_para_errores_500.yaml (3)
767-772: Good error handling coverage for invalid captcha.The test properly captures the error response for invalid captcha input, which is important for testing error handling paths.
848-889: Good error handling coverage for server errors.The test properly captures the 500 Internal Server Error response, which is important for testing error handling paths.
3-3:Details
❓ Verification inconclusive
Verify the test data validity.
The test requests use future dates (2024-2025) which may cause issues if the service validates transaction dates. Consider using dates relative to the current date or documenting why future dates are used.
Also applies to: 19-19, 36-36, 181-181, 214-214, 359-359, 392-392, 537-537, 570-570, 715-715, 748-748
🏁 Script executed:
#!/bin/bash # Search for date validation logic in the codebase rg -A 5 "fecha.*valid"Length of output: 1201
Attention: Please verify the validity of the test request dates in your cassette files. In the file
tests/cassettes/test_lanza_cep_error_para_errores_500.yaml(line 3), the request body usesfecha=08-11-2024, which seems to be a future date relative to other tests. For example, the cassette intests/cassettes/test_fail_validar_transferencia.yamlusesfecha=01-01-2019. If the CEP service enforces strict date validation, this discrepancy could lead to inconsistent test behavior. Consider one of the following approaches:
- Use dates relative to the current date or a fixed past date for consistency across tests.
- Add clear documentation/comments explaining the rationale for using future dates in this particular cassette.
tests/cassettes/test_http_error.yaml (1)
15-73: LGTM! The HTTP error test case is well structured.The test case correctly simulates a chain of redirects from HTTP to HTTPS and handles the 404 error case appropriately.
tests/cassettes/test_validar_transferencia_tipo_35.yaml (1)
3-3: Consider using environment variables for sensitive test data.Similar to the previous file, consider using environment variables for sensitive test data.
Also applies to: 36-36
tests/cassettes/test_validar_transferencia_tipo_12.yaml (1)
3-3: Consider using environment variables for sensitive test data.Similar to the previous files, consider using environment variables for sensitive test data.
Also applies to: 36-36
tests/cassettes/test_validar_transferencia_tipo_36.yaml (2)
3-3: Consider using environment variables for sensitive test data.Similar to the previous files, consider using environment variables for sensitive test data.
Also applies to: 36-36
1-183:Details
❓ Verification inconclusive
Verify HTTPS certificate validation in the test environment.
The test cassettes show HTTP to HTTPS redirects. Ensure that the test environment properly validates HTTPS certificates to prevent security vulnerabilities.
🏁 Script executed:
#!/bin/bash # Description: Check if the test environment enforces HTTPS certificate validation # Search for SSL/TLS verification settings in the test configuration rg -i "verify.*ssl|verify.*cert|ssl.*verify|cert.*verify" tests/Length of output: 66
Action: Manually Verify HTTPS Certificate Validation in the Test Environment
It appears that your test cassettes show HTTP-to-HTTPS redirects without any visible configuration for SSL/certificate verification. Our search in the tests directory did not reveal explicit settings enforcing HTTPS certificate validation. Please manually verify that the test environment (e.g., SSL configurations in test clients or network settings) properly validates certificates to prevent potential security vulnerabilities.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
cep/transferencia.py (2)
65-69: Consider enhancing error handling for XML parsing.While the type casting improves type safety, consider wrapping the XML element access in a try-except block to handle potential parsing errors more gracefully.
- ordenante_element = cast(etree._Element, resp.find('Ordenante')) - beneficiario_element = cast(etree._Element, resp.find('Beneficiario')) - - ordenante = Cuenta.from_etree(ordenante_element) - beneficiario = Cuenta.from_etree(beneficiario_element) + try: + ordenante_element = cast(etree._Element, resp.find('Ordenante')) + beneficiario_element = cast(etree._Element, resp.find('Beneficiario')) + ordenante = Cuenta.from_etree(ordenante_element) + beneficiario = Cuenta.from_etree(beneficiario_element) + except (AttributeError, TypeError) as exc: + raise CepError("Failed to parse account information") from exc
152-156: Consider using a more robust error detection approach.The current error detection might be fragile as it relies on string matching. Consider:
- Using a more specific error code or header from the response.
- Standardizing error detection across different response encodings.
- if ( - NOT_FOUND_ERROR_MESSAGE in resp.decode('utf-8') - or b'no encontrada' in resp - ): + response_text = resp.decode('utf-8') + if any( + error_msg in response_text + for error_msg in [NOT_FOUND_ERROR_MESSAGE, 'no encontrada'] + ):README.md (1)
58-58: Fix heading formatting.Remove trailing colons from headings to comply with Markdown style guidelines:
-### Required Parameters: +### Required Parameters -### Optional Parameters: +### Optional ParametersAlso applies to: 66-66
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
58-58: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(2 hunks)cep/transferencia.py(6 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
58-58: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
66-66: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🔇 Additional comments (5)
cep/transferencia.py (3)
3-4: LGTM! Good improvements to type safety and error handling.
- Using
Decimalinstead offloatfor monetary values prevents floating-point precision issues.- Type casting with
castimproves type safety when working with lxml elements.- Well-defined error message for the new
NotFoundError.Also applies to: 12-12, 19-21
26-27: LGTM! Improved type definitions and data model.The changes align with the expected data model:
fecha_operacionis now correctly typed asdate.fecha_abonoproperly represents the credit date and time.montoandivauseDecimalfor precise monetary calculations.Also applies to: 30-31, 37-38
116-116: Verify the date field used for validation.Using
fecha_abono.date()instead offecha_operacionfor validation might cause issues if the credit date differs from the operation date. Please verify which date field should be used for validation.README.md (2)
5-5: LGTM! Consistent package renaming.The package name has been updated consistently in both the PyPI badge and installation command.
Also applies to: 13-13
16-29: LGTM! Helpful development setup instructions.The new section provides valuable information for developers:
- Clear instructions for using the staging environment
- Simple pytest setup for running tests
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
README.md (5)
7-7: Format the bare URL as a markdown link.Convert the bare URL to a proper markdown link for better readability and to follow markdown best practices.
-Python client library for CEP (http://www.banxico.org.mx/cep/) +Python client library for CEP ([http://www.banxico.org.mx/cep/](http://www.banxico.org.mx/cep/))🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Bare URL used
null(MD034, no-bare-urls)
18-24: Add a note about the staging environment's availability.Consider adding a disclaimer about the staging environment's availability and potential changes to its URL, as this information is crucial for developers.
You can use a staging environment to test the library: ```python from cep import Config Config.BASE_URL = 'http://www.banxico.org.mx/cep-beta'+> Note: The staging environment URL may change. Please refer to the official documentation or contact support if you experience any issues.
--- `50-51`: **Maintain consistent language in documentation.** The error message is in Spanish while the rest of the documentation is in English. Consider translating it for consistency. ```diff - print('No se encontro la transferencia') + print('Transfer not found')
58-58: Remove trailing colons from headers.Remove the trailing colons from section headers to follow markdown best practices.
-### Required Parameters: +### Required Parameters -### Optional Parameters: +### Optional ParametersAlso applies to: 66-66
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
58-58: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
76-78: Enhance method signature documentation.Consider showing the complete method signature with the default format parameter for better clarity.
```python +# tr.descargar(formato: str = 'PDF') -> bytes tr.descargar(formato='XML')</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between bfa17d35e56c47a00f615aaa1971351fb70d10c3 and d69d13c9d5c76b649402ca25d401638341b65ab5. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `README.md` (1 hunks) * `cep/version.py` (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * cep/version.py </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>README.md</summary> 7-7: Bare URL used null (MD034, no-bare-urls) --- 58-58: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) --- 66-66: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) </details> </details> </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>README.md (1)</summary> `64-64`: **Align parameter type documentation with example.** The `monto` parameter is documented as `Decimal` type, but the example shows a float value (8.17). Either update the documentation to reflect the accepted types or update the example to use `Decimal`. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
tests/test_transferencia.py (1)
56-56: Rename unused loop variable.The loop control variable
iis not used within the loop body.Apply this diff to fix the issue:
- for i in range(10): + for _i in range(10):🧰 Tools
🪛 Ruff (0.8.2)
56-56: Loop control variable
inot used within loop bodyRename unused
ito_i(B007)
README.md (1)
7-7: Fix Markdown formatting issues.
- Use link syntax for the bare URL.
- Remove trailing colons from headings.
Apply these diffs to fix the issues:
-Python client library for CEP (http://www.banxico.org.mx/cep/) +Python client library for CEP ([http://www.banxico.org.mx/cep/](http://www.banxico.org.mx/cep/)) -### Required Parameters: +### Required Parameters -### Optional Parameters: +### Optional ParametersAlso applies to: 58-58, 66-66
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Bare URL used
null(MD034, no-bare-urls)
tests/cassettes/test_fail_validar_transferencia_operacion.yaml (1)
35-130: Detailed HTML Response Captured
The second interaction provides a comprehensive HTML response that mimics the error scenario (i.e. transfer not found). The structure is detailed and covers metadata, styling, and script inclusions. As a suggestion, you might consider using a YAML literal block (|) for embedding large HTML content so that the cassette remains more readable and easier to maintain.tests/cassettes/test_validar_transferencia_encontrada_sin_cep.yaml (1)
35-307: Comprehensive HTML Response for Transfer Details
The second interaction returns a detailed HTML response containing a data table and descriptive payment state information. This rich response is well captured; however, for improved readability and easier future modifications, consider formatting the multi-line HTML using a YAML literal block (|). This change would help maintain clarity in the cassette file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(1 hunks)cep/exc.py(1 hunks)cep/transferencia.py(6 hunks)tests/cassettes/test_fail_validar_transferencia_operacion.yaml(1 hunks)tests/cassettes/test_fail_validar_transferencia_pago.yaml(1 hunks)tests/cassettes/test_validar_transferencia_encontrada_sin_cep.yaml(1 hunks)tests/test_transferencia.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cep/exc.py
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~82-~82: Loose punctuation mark.
Context: ...') ``` ## Exceptions - NotFoundError: The transfer was not found. - `MaxReque...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~83-~83: Loose punctuation mark.
Context: ...nsfer was not found. - MaxRequestError: The maximum number of requests has been...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~84-~84: Loose punctuation mark.
Context: ...s been reached. - CepNotAvailableError: The transfer was found, but the CEP is ...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
README.md
7-7: Bare URL used
null
(MD034, no-bare-urls)
58-58: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
66-66: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🪛 Ruff (0.8.2)
tests/test_transferencia.py
56-56: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
🔇 Additional comments (10)
cep/transferencia.py (4)
3-4: LGTM! Good improvements to error handling and financial calculations.
- Using
Decimalinstead offloatis more appropriate for financial calculations to avoid floating-point precision issues.- Error messages are well-defined and descriptive for different scenarios.
Also applies to: 19-30
35-47: LGTM! Good improvements to data types and field separation.
- Separating
fecha_operacion(date) andfecha_abono(datetime) improves clarity.- Using
Decimalfor monetary values (monto,iva) ensures precision.- Added
tipo_pagoandpago_a_bancofor better payment type handling.
74-102: LGTM! Good improvements to type safety and data extraction.
- Type casting with
cast(etree._Element, ...)improves type safety.- Extraction of
fecha_abonofromcadenaCDAis well-documented.- Added handling of
tipo_pagofromcadenaCDA.
161-166: LGTM! Good improvements to error handling.
- Added specific error handling for different scenarios:
CepNotAvailableErrorwhen CEP is not available.NotFoundErrorwhen payment or operation is not found.tests/test_transferencia.py (2)
17-93: LGTM! Good test coverage for error scenarios.Test cases cover all error scenarios:
- Payment not found
- Operation not found
- Transfer found but CEP not available
🧰 Tools
🪛 Ruff (0.8.2)
56-56: Loop control variable
inot used within loop bodyRename unused
ito_i(B007)
97-314: LGTM! Excellent test coverage for payment types.Added comprehensive test cases for different payment types (1-36), validating:
- Transfer object creation
- Payment type assignment
- Account details based on payment type
README.md (1)
16-85: LGTM! Excellent documentation updates.
- Added clear development & testing instructions
- Added comprehensive usage examples with error handling
- Added detailed parameter documentation
- Added clear exceptions documentation
🧰 Tools
🪛 LanguageTool
[uncategorized] ~82-~82: Loose punctuation mark.
Context: ...') ``` ## Exceptions -NotFoundError: The transfer was not found. - `MaxReque...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~83-~83: Loose punctuation mark.
Context: ...nsfer was not found. -MaxRequestError: The maximum number of requests has been...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~84-~84: Loose punctuation mark.
Context: ...s been reached. -CepNotAvailableError: The transfer was found, but the CEP is ...(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
58-58: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
66-66: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
tests/cassettes/test_fail_validar_transferencia_pago.yaml (1)
1-82: LGTM! Good test data for payment not found scenario.The VCR cassette correctly captures:
- HTTP to HTTPS redirect
- Error response for payment not found
tests/cassettes/test_fail_validar_transferencia_operacion.yaml (1)
1-34: Well-structured Redirect Interaction
The first interaction correctly simulates an HTTP POST request that receives a 307 Temporary Redirect response. The request’s parameters, headers, and expected redirect header are clearly defined.tests/cassettes/test_validar_transferencia_encontrada_sin_cep.yaml (1)
1-34: Accurate Redirect Handling for Found Transfer
The initial interaction accurately mimics the API behavior by issuing an HTTP POST that results in a 307 redirect. All key parameters and headers are included to reflect the Banxico service’s expected handling.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
cep/transferencia.py (1)
161-167: Consider consolidating error message checks.The current implementation checks error messages separately. This could be simplified and made more maintainable.
Consider using a mapping:
+ERROR_MESSAGES = { + NO_CEP_ERROR_MESSAGE: CepNotAvailableError, + NO_PAYMENT_ERROR_MESSAGE: TransferNotFoundError, + NO_OPERATION_ERROR_MESSAGE: TransferNotFoundError, +} - if NO_CEP_ERROR_MESSAGE in decoded_resp: - raise CepNotAvailableError - if ( - NO_PAYMENT_ERROR_MESSAGE in decoded_resp - or NO_OPERATION_ERROR_MESSAGE in decoded_resp - ): - raise TransferNotFoundError + for message, exception in ERROR_MESSAGES.items(): + if message in decoded_resp: + raise exceptiontests/test_transferencia.py (2)
56-56: Fix unused loop variable.The loop control variable
iis not used within the loop body.- for i in range(10): + for _ in range(10):🧰 Tools
🪛 Ruff (0.8.2)
56-56: Loop control variable
inot used within loop bodyRename unused
ito_i(B007)
96-315: Consider parameterizing transfer type tests.The test cases for different transfer types share similar structure and assertions. Consider using
pytest.mark.parametrizeto reduce code duplication and make the tests more maintainable.Example refactor:
@pytest.mark.parametrize( 'test_params', [ { 'fecha': dt.date(2024, 11, 8), 'clave_rastreo': 'BiB202411081016248360', 'emisor': '37166', 'receptor': '90723', 'cuenta': '723969000011000077', 'monto': 3414.95, 'tipo_pago': 1, 'assertions': lambda tr: [ (tr.tipo_pago, 1), # Add other type-specific assertions ] }, # Add other test cases ] ) def test_validar_transferencia_tipos(test_params): tr = Transferencia.validar( fecha=test_params['fecha'], clave_rastreo=test_params['clave_rastreo'], emisor=test_params['emisor'], receptor=test_params['receptor'], cuenta=test_params['cuenta'], monto=test_params['monto'], ) assert tr is not None for assertion in test_params['assertions'](tr): assert assertion[0] == assertion[1]README.md (2)
7-7: Fix bare URL in documentation.Replace the bare URL with a proper Markdown link.
-Python client library for CEP (http://www.banxico.org.mx/cep/) +Python client library for CEP ([http://www.banxico.org.mx/cep/](http://www.banxico.org.mx/cep/))🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Bare URL used
null(MD034, no-bare-urls)
58-58: Fix heading punctuation.Remove trailing colons from headings to comply with Markdown style guidelines.
-### Required Parameters: +### Required Parameters -### Optional Parameters: +### Optional ParametersAlso applies to: 66-66
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
58-58: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
README.md(1 hunks)cep/__init__.py(1 hunks)cep/client.py(2 hunks)cep/exc.py(1 hunks)cep/transferencia.py(6 hunks)tests/cassettes/test_validar_transferencia_tipo_1.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_10.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_11.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_12.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_3.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_30.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_31.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_35.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_36.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_4.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_5.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_6.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_8.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_9.yaml(1 hunks)tests/conftest.py(1 hunks)tests/test_transferencia.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (15)
- tests/cassettes/test_validar_transferencia_tipo_1.yaml
- cep/init.py
- tests/cassettes/test_validar_transferencia_tipo_5.yaml
- tests/cassettes/test_validar_transferencia_tipo_10.yaml
- tests/cassettes/test_validar_transferencia_tipo_11.yaml
- tests/cassettes/test_validar_transferencia_tipo_8.yaml
- tests/cassettes/test_validar_transferencia_tipo_36.yaml
- tests/cassettes/test_validar_transferencia_tipo_6.yaml
- tests/cassettes/test_validar_transferencia_tipo_3.yaml
- tests/cassettes/test_validar_transferencia_tipo_31.yaml
- tests/cassettes/test_validar_transferencia_tipo_30.yaml
- tests/cassettes/test_validar_transferencia_tipo_35.yaml
- tests/cassettes/test_validar_transferencia_tipo_9.yaml
- tests/cassettes/test_validar_transferencia_tipo_4.yaml
- tests/cassettes/test_validar_transferencia_tipo_12.yaml
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~82-~82: Loose punctuation mark.
Context: ...## Exceptions - TransferNotFoundError: The transfer was not found. - `MaxReque...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~83-~83: Loose punctuation mark.
Context: ...nsfer was not found. - MaxRequestError: The maximum number of requests has been...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~84-~84: Loose punctuation mark.
Context: ...s been reached. - CepNotAvailableError: The transfer was found, but the CEP is ...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
README.md
7-7: Bare URL used
null
(MD034, no-bare-urls)
58-58: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
66-66: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🪛 Ruff (0.8.2)
tests/test_transferencia.py
56-56: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
🔇 Additional comments (10)
cep/exc.py (1)
8-12: LGTM! Well-structured exception hierarchy.The new exception classes are well-documented and follow a clear hierarchy. The names and docstrings accurately describe their purpose:
TransferNotFoundError: When transfer is not foundCepNotAvailableError: When transfer is found but CEP is not availableAlso applies to: 22-26
cep/client.py (2)
10-15: LGTM! Clean configuration management.The new configuration system with constants and a
configurefunction improves maintainability and follows the established pattern from cuenca-python.
19-19: Consider thread safety implications of modifying class variables.The
base_urlclass variable is modified by theconfigurefunction and accessed in therequestmethod. This could lead to race conditions in a multi-threaded environment where different threads need different base URLs.Consider one of these approaches:
- Document that the configuration is not thread-safe and should be set at startup.
- Use thread-local storage for the base URL.
- Make the base URL an instance variable initialized from the class variable.
Example of approach #3:
class Client: base_url: ClassVar[str] = BASE_URL def __init__(self): + self._base_url = Client.base_url self.session = requests.Session() self.session.headers['User-Agent'] = USER_AGENT self.base_data = dict( tipoCriterio='T', captcha='c', tipoConsulta=1, ) def request( self, method: str, endpoint: str, data: dict, **kwargs ) -> bytes: - url = Client.base_url + endpoint + url = self._base_url + endpointAlso applies to: 40-40
tests/conftest.py (1)
8-9: LGTM! Clear test configuration.Configuring the beta endpoint for testing is a good practice that isolates tests from production.
cep/transferencia.py (1)
40-52: LGTM! Improved type safety and field clarity.Good improvements:
- Using
Decimalfor monetary values prevents floating-point issues- Separating
fecha_operacion(date) andfecha_abono(datetime) improves clarity- Adding
ivaandtipo_pagofields provides more complete informationtests/test_transferencia.py (3)
5-5: LGTM! Good import organization.The imports are well-organized, with specific imports that align with best practices.
Also applies to: 8-13
17-26: LGTM! Comprehensive error handling test cases.The test cases thoroughly cover different error scenarios including invalid transfers and unavailable CEPs.
Also applies to: 30-39, 84-93
43-50: LGTM! Good test coverage for PDF download and rate limiting.The test cases effectively verify PDF content matching and rate limit handling.
Also applies to: 71-80
README.md (2)
5-5: LGTM! Clear installation instructions.The package rename is clearly documented with updated installation commands.
Also applies to: 12-14
31-52: LGTM! Well-documented usage example.The example clearly demonstrates proper usage with error handling.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
tests/test_transferencia.py (1)
56-56: Fix unused loop variable.The loop control variable
iis not used within the loop body.Apply this diff:
- for i in range(10): + for _ in range(10):🧰 Tools
🪛 Ruff (0.8.2)
56-56: Loop control variable
inot used within loop bodyRename unused
ito_i(B007)
README.md (2)
7-7: Fix bare URL in documentation.Replace the bare URL with a proper markdown link.
Apply this diff:
-Python client library for CEP (http://www.banxico.org.mx/cep/) +Python client library for CEP ([http://www.banxico.org.mx/cep/](http://www.banxico.org.mx/cep/))🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Bare URL used
null(MD034, no-bare-urls)
58-68: Fix heading formatting.Remove trailing colons from headings.
Apply this diff:
-### Required Parameters: +### Required Parameters -### Optional Parameters: +### Optional Parameters🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
58-58: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
66-66: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
tests/cassettes/test_validar_transferencia_tipo_1.yaml (1)
81-115: Review of GET Response in Interaction 2
The XML response contains detailed transaction information (including attributes likeFechaOperacion,Hora, andClaveSPEI), which is key for downstream validations. One suggestion is to consider using YAML’s block scalar notation (using|) for the long multiline string to improve readability and maintainability.tests/cassettes/test_validar_transferencia_tipo_36.yaml (1)
81-117: Review of GET Response in Interaction 2
The XML response for transfer type 36 is comprehensive—it contains all the expected elements such as transaction timestamps, certificate numbers, and beneficiary/order details. For improved maintainability, consider refactoring this very long inline string into a YAML literal block (using the|symbol). This could enhance readability and reduce potential formatting issues over time.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
README.md(1 hunks)cep/transferencia.py(6 hunks)cep/version.py(1 hunks)tests/cassettes/test_validar_transferencia_tipo_1.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_10.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_11.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_12.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_3.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_30.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_31.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_35.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_36.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_4.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_5.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_6.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_8.yaml(1 hunks)tests/cassettes/test_validar_transferencia_tipo_9.yaml(1 hunks)tests/conftest.py(1 hunks)tests/test_transferencia.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- cep/version.py
- tests/cassettes/test_validar_transferencia_tipo_31.yaml
- tests/cassettes/test_validar_transferencia_tipo_11.yaml
- tests/cassettes/test_validar_transferencia_tipo_8.yaml
- tests/cassettes/test_validar_transferencia_tipo_5.yaml
- tests/cassettes/test_validar_transferencia_tipo_3.yaml
- tests/cassettes/test_validar_transferencia_tipo_30.yaml
- tests/cassettes/test_validar_transferencia_tipo_9.yaml
- tests/cassettes/test_validar_transferencia_tipo_6.yaml
- tests/cassettes/test_validar_transferencia_tipo_4.yaml
- tests/cassettes/test_validar_transferencia_tipo_10.yaml
- tests/cassettes/test_validar_transferencia_tipo_35.yaml
- tests/cassettes/test_validar_transferencia_tipo_12.yaml
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~82-~82: Loose punctuation mark.
Context: ...## Exceptions - TransferNotFoundError: The transfer was not found. - `MaxReque...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~83-~83: Loose punctuation mark.
Context: ...nsfer was not found. - MaxRequestError: The maximum number of requests has been...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~84-~84: Loose punctuation mark.
Context: ...s been reached. - CepNotAvailableError: The transfer was found, but the CEP is ...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
README.md
7-7: Bare URL used
null
(MD034, no-bare-urls)
58-58: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
66-66: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🪛 Ruff (0.8.2)
tests/test_transferencia.py
56-56: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
🔇 Additional comments (14)
tests/conftest.py (2)
20-21: Avoid hardcoded future dates in tests.The test data uses dates from November 2024, which could cause tests to fail after these dates pass.
Apply this diff to use relative dates:
- fecha_operacion=datetime.date(2024, 11, 8), - fecha_abono=datetime.datetime(2024, 11, 8, 10, 53, 36), + fecha_operacion=datetime.date.today(), + fecha_abono=datetime.datetime.now().replace(microsecond=0),
18-53: LGTM! The fixture updates improve clarity and align with class changes.The renaming to
transferencia_tipo_1and the addition of new fields likeiva,tipo_pago, etc., make the test data more explicit and match the updatedTransferenciaclass.cep/transferencia.py (4)
24-35: LGTM! Error messages are well-defined and descriptive.The error messages are clearly defined as constants and provide specific information about different error scenarios.
38-52: LGTM! Type changes improve precision and clarity.
- Changing
montotoint(cents) prevents floating-point precision issues with monetary values.- Using
dateforfecha_operacionanddatetimeforfecha_abonoclearly distinguishes between date-only and timestamp fields.- New attributes are well-typed and documented.
83-87: Improve type casting safety.The current implementation uses
castto handle potentialNonevalues fromfind, but this could mask real issues.Consider explicit None checks:
- ordenante_element = cast(etree._Element, resp.find('Ordenante')) - beneficiario_element = cast(etree._Element, resp.find('Beneficiario')) - - ordenante = Cuenta.from_etree(ordenante_element) - beneficiario = Cuenta.from_etree(beneficiario_element) + ordenante_element = resp.find('Ordenante') + beneficiario_element = resp.find('Beneficiario') + if ordenante_element is None or beneficiario_element is None: + raise CepError("Missing required elements in response") + + ordenante = Cuenta.from_etree(ordenante_element) + beneficiario = Cuenta.from_etree(beneficiario_element)
156-174: LGTM! Error handling and parameter handling improvements.
- Proper conversion of monto from cents to pesos.
- Clear error handling with specific exceptions.
- Added support for pago_a_banco parameter.
tests/test_transferencia.py (1)
96-314: LGTM! Comprehensive test coverage for all transfer types.The tests:
- Cover all transfer types (1-36).
- Verify specific attributes for each type.
- Include error cases and edge conditions.
README.md (1)
10-85: LGTM! Documentation is comprehensive and well-structured.The documentation:
- Provides clear installation and development instructions.
- Includes detailed usage examples with error handling.
- Documents all parameters and exceptions.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~82-~82: Loose punctuation mark.
Context: ...## Exceptions -TransferNotFoundError: The transfer was not found. - `MaxReque...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~83-~83: Loose punctuation mark.
Context: ...nsfer was not found. -MaxRequestError: The maximum number of requests has been...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~84-~84: Loose punctuation mark.
Context: ...s been reached. -CepNotAvailableError: The transfer was found, but the CEP is ...(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
58-58: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
66-66: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
tests/cassettes/test_validar_transferencia_tipo_1.yaml (3)
1-20: Review of POST Request in Interaction 1
The request block for the validation endpoint is well defined. The query-string parameters (e.g.,tipoCriterio=T,captcha=c,tipoConsulta=1, etc.) appear correct and are in line with the expected test values.
21-61: Review of POST Response in Interaction 1
The HTML response body is comprehensive—it includes the expected download options (PDF, XML, ZIP) along with the proper structure. Please verify in your test assertions that any dynamic values (such as date/time stamps or cookie values) are tolerated or normalized appropriately.
62-80: Review of GET Request in Interaction 2
The GET request block correctly uses anullbody and provides the necessary headers including cookies for session management. This structure will help simulate a realistic client request.tests/cassettes/test_validar_transferencia_tipo_36.yaml (3)
1-20: Review of POST Request in Interaction 1
The POST request for transfer type 36 is defined with the correct parameters—note the updated criterion value (criterio=BXM492411081919171201) and corresponding numerical values. The endpoint URI is accurate, ensuring that this interaction targets the correct validation service.
20-61: Review of POST Response in Interaction 1
The HTML response is detailed and mirrors the structure of the previous cassette with download links and proper styling. It is important to double-check that the content length and cookies match those expected in a production scenario.
62-80: Review of GET Request in Interaction 2
This request block correctly sets the body asnulland includes the necessary headers, including cookies and the user agent string. Such consistency is vital for session continuity in your tests.
Description
This PR updates GitHub Actions, adds support for Python 3.10-3.13, and removes support for 3.7 and 3.8 and upgrades clabe to version 2.0.0, which now uses Pydantic v2
Changes
clabeto version 2.0.0, which now usesPydantic v2.Summary by CodeRabbit
New Features
Documentation
Improvements