-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/new api integration #9
Conversation
WalkthroughThe repository now includes two new GitHub workflows: one automates package publishing to PyPI on release events (using ✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (1)
tests/test_person.py (1)
20-33
: 🛠️ Refactor suggestionReplace sensitive test data with mock data.
Similar to the previous test, replace the real-looking CURP and RFC with clearly fake test data.
🧹 Nitpick comments (14)
quienesquien/client.py (2)
16-20
: Avoid embedding credentials directly in code.
Defining attributes like secret_key can pose security risks. It’s often better to load credentials from environment variables or a secure config store.
37-39
: Consider URL-encoding parameters.
Concatenating fields into the URL works, but be aware of potential issues with spaces or special characters in nombre/paterno/materno. Using something like urllib.parse.urlencode would be safer for query strings.tests/conftest.py (1)
5-8
: Hardcoded credentials for tests.
While acceptable for local tests or mocks, consider environment variables or secrets management if these tests run in CI/CD to avoid exposing sensitive info.setup.cfg (1)
20-23
: Consider adding coverage thresholds.While the current coverage configuration is good, consider adding minimum coverage thresholds to maintain code quality.
[coverage:report] precision = 2 exclude_lines = if TYPE_CHECKING: +fail_under = 80
tests/test_client.py (2)
4-9
: Consider adding more edge cases.While the current test is good, consider adding tests for:
- Empty string inputs
- Special characters in names
- Very long names
12-17
: Enhance assertions for more specific validation.Consider adding assertions to validate:
- Exact number of records
- Structure of returned person objects
- Specific field values in the response
setup.py (1)
1-5
: Consider using a simpler version loading approach.While
SourceFileLoader
works, consider usingimportlib.metadata
for a more modern approach to version management.-from importlib.machinery import SourceFileLoader +from importlib.metadata import version from setuptools import find_packages, setup -version = SourceFileLoader('version', 'quienesquien/version.py').load_module() +__version__ = version('quienesquien')quienesquien/person.py (1)
1-66
: Document the purpose of thecoincidencia
field.The
coincidencia
field has been added toPERSON_FIELDNAMES
, but its purpose and expected values are not documented. Consider adding docstrings or comments to explain what this field represents and how it's used in the API integration.README.md (1)
7-7
: Fix markdown lint issue: Wrap bare URL in angle brackets.To comply with markdown best practices, wrap the URL in angle brackets.
-Cliente para el servicio de listas de Quienesquien (https://app.q-detect.com/) +Cliente para el servicio de listas de Quienesquien (<https://app.q-detect.com/>)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Bare URL used
null(MD034, no-bare-urls)
Makefile (3)
5-6
: Formatting Command Definitions
The isort and black commands are clearly specified with a target version (py313) and a custom line-length flag (-l 79). Verify that the 79-character limit is intentional and compatible with the team’s style guide.
34-48
: Clean Target Considerations
The clean target comprehensively removes cache and temporary files. Note that using backticks in rm commands can sometimes lead to issues with filenames containing spaces; consider using $(shell find …) if this becomes a concern.
49-52
: Release Target Consistency
The release target currently uses a bare “python” call for packaging, which might be inconsistent with the rest of the Makefile that uses the defined PYTHON variable. For consistency, it’s suggested to use $(PYTHON) here.Proposed diff:
-release: test clean - python setup.py sdist bdist_wheel - twine upload dist/* +release: test clean + $(PYTHON) setup.py sdist bdist_wheel + twine upload dist/*tests/cassettes/test_search_not_found.yaml (1)
21-50
: Not Found Search Interaction
The second interaction simulates a search that yields no results. The request and expected JSON response (with success: false and a status message in Spanish) are clearly defined. Consider whether dynamic token handling might be more robust during live testing versus a hard-coded value in the cassette.tests/cassettes/test_search.yaml (1)
21-113
: Detailed Search Interaction with Successful Result
This interaction simulates a successful search, returning a JSON response with multiple user records and various fields. Although the JSON string is long and spans multiple lines, it appears to capture a realistic API response. Ensure that all essential fields are correctly parsed in tests and that special characters/escapes (e.g., in names with accents) are handled properly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.github/workflows/release.yml
(1 hunks).github/workflows/test.yml
(1 hunks)Makefile
(1 hunks)README.md
(1 hunks)quienesquien/__init__.py
(1 hunks)quienesquien/client.py
(1 hunks)quienesquien/person.py
(1 hunks)quienesquien/version.py
(1 hunks)requirements-test.txt
(1 hunks)requirements.txt
(1 hunks)setup.cfg
(1 hunks)setup.py
(1 hunks)test_quienesquien.py
(0 hunks)test_search
(0 hunks)test_search_not_found
(0 hunks)tests/cassettes/test_search.yaml
(1 hunks)tests/cassettes/test_search_not_found.yaml
(1 hunks)tests/conftest.py
(1 hunks)tests/test_client.py
(1 hunks)tests/test_person.py
(1 hunks)
💤 Files with no reviewable changes (3)
- test_quienesquien.py
- test_search_not_found
- test_search
✅ Files skipped from review due to trivial changes (2)
- quienesquien/version.py
- requirements.txt
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
7-7: Bare URL used
null
(MD034, no-bare-urls)
🔇 Additional comments (32)
quienesquien/client.py (10)
1-2
: Ensure Python version compatibility for union types.
You're using the union operator (|) for types (e.g., int | bool in line 10). That syntax requires Python 3.10+. Confirm that your environment meets this requirement or adjust if you need wider compatibility.
6-6
: Relative import looks good.
No issues here; the relative import from .person is appropriate within a well-structured package.
9-11
: TypedDict usage is well-structured.
Defining SearchResult as a TypedDict clarifies the shape of the response object, improving type safety.
14-14
: Good use of @DataClass.
Using @DataClass simplifies boilerplate for class initialization and improves readability.
22-23
: Docstring is clear and concise.
It accurately describes the purpose of the_fetch_auth_token
method.
24-30
: Verify token extraction.
Returning response.text assumes the token is delivered plainly. If the API returns JSON or a different format, you might need to parse the value more specifically.
32-36
: Search method initialization looks good.
Fetching the auth token and preparing initial parts of the search request is straightforward and well-organized.
42-44
: Sufficient basic error handling with raise_for_status().
This ensures HTTP error codes won't be silently ignored.
46-55
: Check for missing fields in the response data.
Filtering out None fields is good, but confirm the upstream API always provides the expected keys. Consider robust error handling if the JSON structure changes.
65-65
: Clean return statement.
Returning the typed dictionary with the shape defined by SearchResult is consistent and clear.quienesquien/__init__.py (1)
1-5
: Export definitions are well-organized.
Listing Client, Person, and version under all clarifies the public interface. Keep each entry in sync with module changes.tests/conftest.py (2)
1-3
: Pytest and Client import
These imports are correct and conventional for setting up test configuration.
11-13
: Clear and reusable fixture.
Using a fixture to yield the Client instance is a good practice, promoting test modularity.setup.cfg (3)
4-5
: LGTM! Good test configuration setup.The pytest configuration with coverage reporting is well-structured and aligns with the PR objectives.
11-15
: LGTM! Good import sorting configuration.The isort configuration follows common practices for organizing imports.
17-18
: LGTM! Good type checking configuration.The mypy configuration correctly handles pytest imports.
setup.py (2)
21-23
: LGTM! Good package configuration.The package configuration correctly includes type hints support with py.typed.
24-31
: Verify package version compatibility.The Python and requests version requirements look good, but let's verify:
- The latest compatible requests version
- Whether Python 3.13 is stable for production use
✅ Verification successful
🌐 Web query:
What is the latest stable version of the requests package?
💡 Result:
The latest stable version of the Python Requests package is **2.32.3**, released on May 21, 2024[1][10]. This version includes security fixes, performance improvements for SSL context reuse, and updated support for Python versions (officially supporting CPython 3.8+ and PyPy 3.9/3.10)[10]. Key details about this release: - **Security fix**: Resolved an issue where `verify=False` on initial requests could improperly disable certificate verification for subsequent same-origin requests[10] - **Deprecations**: Dropped support for Python 3.7 and PyPy 3.7/3.8[10] - **Improvements**: Added global SSLContext reuse for faster subsequent requests[10] The Requests project maintains regular releases - there have been 5 updates since mid-2024 alone[10]. You can install/upgrade using: ```bash pip install --upgrade requests ``` Citations: - 1: https://requests.readthedocs.io/en/latest/ - 2: https://www.scrapingbee.com/webscraping-questions/requests/is-requests-a-built-in-python-library/ - 3: https://proxiesapi.com/articles/are-python-requests-deprecated - 4: https://www.activestate.com/resources/quick-reads/how-to-pip-install-requests-python-package/ - 5: https://docs.python-requests.org/en/latest/community/release-process/ - 6: https://pypi.org/project/requests/2.11.0/ - 7: https://www.npmjs.com/package/request - 8: https://pypi.org/project/requests/2.7.0/ - 9: https://github.com/psf/requests/blob/main/HISTORY.md - 10: https://github.com/psf/requests/releases
🌐 Web query:
Is Python 3.13 stable for production use?
💡 Result:
Python 3.13 has entered its **bugfix phase** with no new feature additions since May 2024[1], making it **officially stable** for production use under standard configurations[1][3]. However, critical considerations remain: ### Stability factors 1. **Core functionality stability** - Passed extensive testing after resolving garbage collector performance issues[1] - Maintains ABI compatibility since beta versions[1] - Now receiving only bug/security fixes under maintenance mode[3] 2. **Experimental feature risks** - Free-threading (GIL-less mode) and JIT compiler remain **unstable for production**[6][10] - Performance inconsistencies observed when enabling/disabling GIL[5] - Requires special compilation flags (`--disable-gil`, `--enable-experimental-jit`)[6] ### Ecosystem readiness - ~30% of PyPI packages lacked 3.13-compatible wheels at launch[2] - Conda-Forge automated updates help but depend on maintainer support[2] - Legacy C extensions might require updates due to API changes[9] ### Version support timeline | Version | Support Status | End of Life | |---------|----------------------|-------------------| | 3.13 | Active bugfixes | October 2029[1] | | 3.12 | Security fixes only | October 2027[3] | | 3.11 | Security fixes only | October 2025[3] | **Recommendation**: Upgrade non-critical systems first while monitoring: - Dependency compatibility matrices - Memory usage patterns (GC changes)[1] - Threading behavior if considering future GIL-less adoption[8] Production deployments should avoid experimental features until Python 3.14+ stabilizes them[6][10]. The standard build is production-ready but requires thorough dependency validation[2][5]. Citations: - 1: https://flyaps.com/blog/update-python-3-13/ - 2: https://pythonspeed.com/articles/upgrade-python-3.13/ - 3: https://devguide.python.org/versions/ - 4: https://discuss.python.org/t/end-of-life-eol-versions-usage/69601 - 5: https://realpython.com/python313-new-features/ - 6: https://realpython.com/python313-free-threading-jit/ - 7: https://discuss.python.org/t/documenting-python-versioning-and-stability-expectations/11090 - 8: https://www.deciphr.ai/podcast/how-much-faster-is-python-313-without-the-gil - 9: https://discuss.python.org/t/python-3-13-alpha-1-contains-breaking-changes-whats-the-plan/37490 - 10: https://www.infoworld.com/article/3552750/get-started-with-the-free-threaded-build-of-python-3-13.html
Package Version Compatibility Verified
The version constraints for the requests package and the Python classifiers in setup.py are appropriate. Specifically:
- The requests version range (>=2.32.0, <3.0.0) is valid given that the latest stable version (2.32.3) falls within this range.
- Python 3.13 is confirmed as stable for production use under standard configurations.
quienesquien/person.py (1)
69-75
: LGTM! The Person class implementation is flexible and maintainable.The constructor accepts both dictionaries and keyword arguments, making it versatile for different initialization scenarios.
requirements-test.txt (1)
1-8
: Verify package versions and Python compatibility.Some version constraints seem to be set to future versions that might not exist yet:
black==25.*
(latest stable is 24.*)- These packages need to be compatible with Python 3.13 which is still in development
Let's check the latest versions and Python compatibility:
✅ Verification successful
🌐 Web query:
What is the latest version of black?
💡 Result:
The latest stable version of **Black** as of February 2025 is **25.1.0**, released on February 7, 2025[3][5]. This version introduces Python 3.13 support and stabilizes several formatting changes under the new 2025 stable style[3]. Key updates in v25.1.0: - New formatting rules for generic function definitions and list items[3] - Improved handling of IPython notebook cells and `with` statements[3] - Performance optimizations in tokenization[3] Earlier versions include: - **24.10.0** (October 2024) with Python 3.13 support and bug fixes[1][3] - **24.8.0** (August 2024) addressing parser regressions and performance improvements[1][2] Package managers like Homebrew have updated to v25.1.0[5], while some IDE integrations still reference older versions like 24.8.0 in their documentation[6][7]. Always verify your installation using `black --version` after updating via `pip install -U black`[2][3]. Citations: - 1: https://black.readthedocs.io - 2: https://pypi.org/project/black/ - 3: https://github.com/psf/black/releases - 4: https://github.com/psf/black/actions - 5: https://formulae.brew.sh/formula/black - 6: https://marketplace.visualstudio.com/items?itemName=ms-python.black-formatter - 7: https://black.readthedocs.io/en/stable/integrations/editors.html
🌐 Web query:
What is the latest stable version of Python?
💡 Result:
The latest stable version of Python as of February 2025 is **Python 3.13.2**, released on February 4, 2025[4]. This version receives regular bug fixes and security updates under its maintenance phase[2][3]. ### Key Details: - **Python 3.13** entered its bugfix phase on October 7, 2024[2], with support planned until October 2029[3]. - **Python 3.12** (latest release: **3.12.9**) remains in bugfix mode until October 2028[2][3]. - **Python 3.11** and older versions are in security-only support phases[1][2]. ### Considerations for Upgrading: - Use Python 3.13 for new features and performance improvements. - Stick with older versions (e.g., **3.12.x** or **3.11.x**) if critical third-party dependencies aren’t yet compatible with newer releases[1]. - Verify Windows compatibility: Versions newer than **Python 3.8** do not support Windows XP/7[4]. For most projects requiring stability and modern features, **Python 3.13.x** is recommended unless dependencies dictate otherwise[1][4]. Citations: - 1: https://discuss.python.org/t/python-latest-stable-version/24563 - 2: https://devguide.python.org/versions/ - 3: https://www.python.org/downloads/ - 4: https://www.python.org/getit/windows/ - 5: https://vocal.media/education/which-python-version-is-best-for-machine-learning - 6: https://www.python.org/doc/versions/
Final Verification: Package Versions and Python Compatibility Confirmed
- The constraint for
black==25.*
is appropriate as version 25.1.0 is the latest stable release as of February 2025.- Python 3.13 is now stable (Python 3.13.2 released in February 2025), ensuring compatibility with the listed packages.
.github/workflows/release.yml (1)
1-24
: LGTM! The release workflow is well structured.The workflow correctly:
- Triggers on release publication
- Uses secure token-based authentication for PyPI
- Includes all necessary steps for building and publishing
README.md (1)
17-30
: LGTM! The example code demonstrates good practices.The example:
- Shows proper error handling
- Includes type checking with
.get()
- Demonstrates how to process the response
Makefile (5)
3-3
: Python Version Update
The PYTHON variable has been updated to use python3.13, which aligns well with the new API and testing requirements.
4-4
: Introduce Project Variable
Defining the PROJECT variable as “quienesquien” improves maintainability and simplifies references in the file.
11-14
: Virtual Environment Creation
The venv target correctly creates a Python virtual environment using the updated PYTHON variable and ensures pip is upgraded. The PATH is pre-pended with “./venv/bin”, so subsequent pip calls will use the venv versions.
15-20
: Installation Targets
The install and install-test targets install production and testing dependencies in sequence. This clear separation is appropriate for ensuring a clean test environment.
21-33
: Test, Format, and Lint Targets
The test target now includes cleaning, testing dependency installation, and linting before running pytest. The format and lint targets integrate isort, black, flake8, and mypy checks to maintain code quality..github/workflows/test.yml (3)
1-7
: Workflow Initialization and Lint Job Setup
The workflow is named “test” and starts on push events. The lint job sets up Python 3.13, installs testing dependencies using “make install-test”, and runs linting via “make lint”. This setup is clear and aligns with the updated Makefile.
19-34
: Pytest Job with Matrix Testing
Great use of a matrix strategy to run tests against multiple Python versions (3.10 through 3.13), ensuring compatibility. The job checks out the code, installs dependencies, and runs pytest.
35-54
: Coverage Job Configuration
The coverage job correctly generates an XML report using “pytest --cov-report=xml” and uploads it using the Codecov action. Ensure that the CODECOV_TOKEN secret is properly configured in the repository settings.tests/cassettes/test_search_not_found.yaml (1)
1-20
: Token Retrieval Interaction
The first interaction simulates a GET request to the token endpoint with the appropriate headers and a 200 OK response. The static token is acceptable for this cassette.tests/cassettes/test_search.yaml (1)
1-20
: Token Retrieval in Successful Search Scenario
The cassette’s first interaction for retrieving the token is similar to the “not found” cassette, ensuring consistency in setup. The headers and expected response are clearly specified.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
quienesquien/client.py (1)
14-20
: Add input validation for the percent parameter.While the default value of 80 for percent is reasonable, consider adding validation to ensure it stays within valid bounds (e.g., 0-100).
@dataclass class Client: base_url: str username: str client_id: str secret_key: str - percent: int = 80 + percent: int = 80 + + def __post_init__(self): + if not 0 <= self.percent <= 100: + raise ValueError("Percent must be between 0 and 100")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
quienesquien/client.py
(1 hunks)tests/cassettes/test_search.yaml
(1 hunks)tests/cassettes/test_search_not_found.yaml
(1 hunks)tests/test_person.py
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/test_person.py
- tests/cassettes/test_search.yaml
- tests/cassettes/test_search_not_found.yaml
🔇 Additional comments (1)
quienesquien/client.py (1)
1-12
: LGTM! Well-structured type definitions.The SearchResult TypedDict provides clear type hints for the API response structure.
e0d3d98
to
daeec2d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (3)
quienesquien/client.py (3)
22-30
: 🛠️ Refactor suggestionEnhance error handling and validate token response.
The method should handle API-specific errors and validate the token format.
32-40
:⚠️ Potential issueAdd URL encoding for search parameters.
Direct string interpolation in URLs can lead to encoding issues or potential security vulnerabilities.
42-65
: 🛠️ Refactor suggestionEnhance error handling and response validation.
While the code handles the happy path well, it should validate the response structure and handle potential errors more robustly.
🧹 Nitpick comments (3)
README.md (2)
7-7
: Convert Bare URL to a Formatted Markdown Link.The line contains a bare URL which might trigger markdown lint warnings (MD034). Consider formatting the URL as a markdown link for improved readability and adherence to markdown best practices.
Apply this diff code:
-Cliente para el servicio de listas de Quienesquien (https://app.q-detect.com/) +Cliente para el servicio de listas de Quienesquien ([https://app.q-detect.com/](https://app.q-detect.com/))🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Bare URL used
null(MD034, no-bare-urls)
15-30
: Usage Section Provides a Concise Example.The "Uso" section demonstrates how to use the Client class effectively. Consider adding a brief comment in the snippet noting that the placeholders (URL, USER, CLIENT_ID, SECRET_ID) should be replaced with actual credentials.
setup.py (1)
1-5
: Add error handling for version loading.While dynamically loading the version is a good practice, consider adding error handling to provide a meaningful error message if the version file is missing or malformed.
-version = SourceFileLoader('version', 'quienesquien/version.py').load_module() +try: + version = SourceFileLoader('version', 'quienesquien/version.py').load_module() +except (ImportError, AttributeError) as e: + raise RuntimeError("Failed to load version from quienesquien/version.py") from e
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/release.yml
(1 hunks).github/workflows/test.yml
(1 hunks)README.md
(1 hunks)quienesquien/client.py
(1 hunks)quienesquien/version.py
(1 hunks)setup.cfg
(1 hunks)setup.py
(1 hunks)tests/cassettes/test_search.yaml
(1 hunks)tests/cassettes/test_search_not_found.yaml
(1 hunks)tests/conftest.py
(1 hunks)tests/test_client.py
(1 hunks)tests/test_person.py
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- tests/test_client.py
- quienesquien/version.py
- tests/cassettes/test_search.yaml
- .github/workflows/release.yml
- .github/workflows/test.yml
- tests/test_person.py
- tests/cassettes/test_search_not_found.yaml
🧰 Additional context used
🪛 GitHub Actions: test
tests/conftest.py
[warning] 22-22: no newline at end of file
🪛 markdownlint-cli2 (0.17.2)
README.md
7-7: Bare URL used
null
(MD034, no-bare-urls)
🔇 Additional comments (14)
README.md (3)
1-1
: Project Title is Correctly Updated.The header now simply states "quienesquien", which aligns well with the updated branding.
3-5
: Badges and Status Indicators Added.The addition of badges for tests, coverage, and PyPI version enhances the visibility of the project’s health and release information.
9-14
: Installation Instructions are Clear.The new "Instalación" section clearly provides the command for installation using pip, making it straightforward for users.
setup.cfg (5)
4-6
: LGTM! Comprehensive test configuration.The pytest configuration includes all necessary options for verbose output, coverage reporting with missing lines, and proper package targeting.
7-10
: Verify quote style consistency across the codebase.The flake8 configuration enforces single quotes for inline strings and triple double quotes for multiline strings.
#!/bin/bash # Description: Check for inconsistent quote usage in Python files. # List Python files echo "Python files in repository:" fd -e py # Check for double quotes in single-line strings echo -e "\nChecking for double quotes in single-line strings (should use single quotes):" rg --pcre2 '"[^"]*"(?!\s*""")' -g '*.py' # Check for single quotes in multiline strings echo -e "\nChecking for triple single quotes in multiline strings (should use triple double quotes):" rg --pcre2 "'''[\s\S]*?'''" -g '*.py'
11-16
: LGTM! Well-structured import sorting configuration.The isort configuration follows best practices for import formatting, ensuring consistent and maintainable code organization.
17-19
: LGTM! Standard mypy configuration for pytest.The configuration correctly handles pytest's dynamic nature by ignoring missing imports.
20-24
: LGTM! Appropriate coverage configuration.The coverage configuration is well-structured with reasonable precision and correctly excludes TYPE_CHECKING blocks.
tests/conftest.py (2)
11-13
: LGTM!The fixture correctly uses pytest's yield pattern and provides a configured Client instance for tests.
16-22
: LGTM!The fixture correctly configures VCR to filter out sensitive Authorization headers, which is a security best practice.
🧰 Tools
🪛 GitHub Actions: test
[warning] 22-22: no newline at end of file
quienesquien/client.py (2)
9-11
: LGTM!The SearchResult TypedDict provides clear type hints that match the API response format.
14-20
: LGTM!The dataclass implementation with clear type hints provides a clean and maintainable interface.
setup.py (2)
24-25
: Verify requests version compatibility with the new API.Since this PR integrates with a new API version, please confirm that the specified requests version range (>=2.32.0,<3.0.0) is compatible with any new API features being used.
✅ Verification successful
🌐 Web query:
What is the latest version of the requests library and what are its minimum Python version requirements?
💡 Result:
The latest stable version of the Python Requests library is **2.32.3**, released on May 29, 2024[4][10]. This version requires **Python 3.8 or newer** according to its official documentation[4], though some sources indicate compatibility starts at **Python 3.7+**[2][3][7]. Key details about compatibility: - Dropped support for Python 2.x in June 2022 (version 2.28+)[7] - Ended Python 3.6 compatibility in June 2022[7] - Actively maintained with regular updates (5-6 minor releases/year)[3][7] The library follows semantic versioning and continues to be one of Python's most popular HTTP clients, with over **30 million weekly downloads**[3]. For new projects, developers should use Python 3.8+ to ensure full compatibility with all modern Requests features and security updates[4][10]. Citations: - 1: https://www.activestate.com/resources/quick-reads/how-to-pip-install-requests-python-package/ - 2: https://www.scrapingbee.com/webscraping-questions/requests/is-requests-a-built-in-python-library/ - 3: https://proxiesapi.com/articles/are-python-requests-deprecated - 4: https://requests.readthedocs.io/en/latest/ - 5: https://pypi.org/project/requests/2.17.3/ - 6: https://discuss.python.org/t/what-is-the-minimum-python3-version-i-can-expect-to-exist-on-most-linux-distributions/76927 - 7: https://github.com/psf/requests/blob/main/HISTORY.md - 8: https://python-forum.io/thread-815.html - 9: https://devguide.python.org/versions/ - 10: https://pypi.org/project/requests/2.11.0/
Requests version configuration verified.
Based on the web query, the latest stable Requests version is 2.32.3, which requires Python 3.8 or newer. Since this package requires Python 3.10, the defined dependency (>=2.32.0,<3.0.0) is not only current but also fully compatible with the new API integration.
31-31
: Verify Python 3.13 compatibility claim.Python 3.13 is still in development. Unless you've tested with Python 3.13 alpha/beta releases, consider removing this classifier until Python 3.13 is officially released.
✅ Verification successful
🌐 Web query:
What is the current status of Python 3.13? Is it released or still in development?
💡 Result:
Python 3.13 has been officially released and is currently in its maintenance phase with multiple updates available since its initial launch[1][5][9]. Here's a detailed breakdown: ### Release Status - **Initial Release**: Python 3.13.0 launched on **October 7, 2024**, after a brief delay from its original October 1 target due to performance issues with an experimental garbage collector[4][6]. - **Latest Maintenance Release**: Python **3.13.2** became available on **February 4, 2025**, addressing ~250 bugs and improvements[1][9]. ### Key Features The 3.13 series introduced several major enhancements: - **Free-threaded Build Mode**: Experimental support for disabling the Global Interpreter Lock (GIL) on Windows/macOS[1][8]. - **Interactive REPL Upgrade**: A PyPy-inspired interpreter with multi-line editing and colorized tracebacks[1][8]. - **Experimental JIT Compiler**: Lays groundwork for future performance gains[1][8]. - **Platform Updates**: - Minimum macOS version raised to **10.13 (High Sierra)**[1] - WASI promoted to Tier 2 support[1] ### Development Timeline - **Final Release Candidate**: Version **3.13.0rc3** (October 1, 2024) preceded the official launch after rolling back problematic garbage collector changes[6]. - **Maintenance Schedule**: Bugfix releases every ~2 months until October 2026, with security updates until **October 2029**[3]. ### Current Recommendations While production use is now viable after multiple maintenance releases[7], developers should: 1. Verify compatibility of critical dependencies 2. Test experimental features like free-threading/JIT in staging environments 3. Use major version aliases (e.g., `python-3.13`) for automatic security updates[5] The version remains supported through at least October 2029 per Python's extended support policies[3][9]. Citations: - 1: https://www.python.org/downloads/release/python-3132/ - 2: https://flyaps.com/blog/update-python-3-13/ - 3: https://peps.python.org/pep-0719/ - 4: https://realpython.com/python-news-october-2024/ - 5: https://devcenter.heroku.com/changelog-items/3139 - 6: https://www.python.org/downloads/release/python-3130rc3/ - 7: https://pythonspeed.com/articles/upgrade-python-3.13/ - 8: https://realpython.com/python313-new-features/ - 9: https://blog.python.org/2025/02/python-3132-and-3129-now-available.html?m=0 - 10: https://www.python.org/downloads/release/python-3130rc2/
Python 3.13 Classifier Verified as Appropriate
The web query confirms that Python 3.13 has been officially released (with version 3.13.0 launched in October 2024 and an update 3.13.2 available as of February 2025) and is in maintenance. Therefore, including the classifier in setup.py is valid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
veo que la documentación de esta nueva versión de QeQ contiene buena especificación de los tipos de respuesta, errores y cosas que pueden ser enums. Esta implementación debe contener este tipo de detalles.
Además de esto hay otro nuevos endpoints que tal vez vale la pena implementar para nuestro uso. Entra dentro del scope de este PR @pachCode ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (1)
quienesquien/client.py (1)
71-75
:⚠️ Potential issueAdd URL encoding for parameters.
Direct string interpolation in URLs can lead to encoding issues.
+from urllib.parse import quote + # Build base URL with required parameters search_url = ( f'{self.base_url}/api/find?client_id={self.client_id}' - f'&username={self.username}&percent={match_score}' - f'&name={nombre} {paterno} {materno}' + f'&username={quote(self.username)}&percent={match_score}' + f'&name={quote(f"{nombre} {paterno} {materno}")}' )📝 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.from urllib.parse import quote # Build base URL with required parameters search_url = ( f'{self.base_url}/api/find?client_id={self.client_id}' f'&username={quote(self.username)}&percent={match_score}' f'&name={quote(f"{nombre} {paterno} {materno}")}' )
🧹 Nitpick comments (5)
requirements.txt (1)
2-2
: Nitpick: Trailing Blank Line
There is an extra blank line at the end of the file. Consider removing it if it does not conform to the project’s file formatting guidelines.README.md (1)
36-80
: Example Code Enhancement for Async Context
The example code effectively demonstrates client instantiation and handling both successful and no-result search scenarios. However, since the code usesawait
on asynchronous calls (lines 47 and 67), it would be beneficial to wrap the snippet inside an asynchronous context (e.g., usingasync def main():
withasyncio.run(main())
). This added context helps clarify that the code must execute in an async environment.quienesquien/person.py (2)
4-5
: Consider adding a docstring to clarify the class purpose.
A short docstring describing thePerson
class would improve clarity and help document usage.
6-69
: Confirm field types and leverage domain-specific types if possible.
All fields are typed asstr | None
orint | None
, but consider using specialized types (e.g.,datetime
for date fields) or enumerations for known string variants to prevent accidental misuse and improve maintainability. If running Python < 3.10, ensure a compatible syntax or addfrom __future__ import annotations
.tests/conftest.py (1)
47-50
: Consider using a more explicit approach to reset the counter.The current implementation uses closure cell contents, which is not very maintainable.
-@pytest.fixture(autouse=True, scope='function') -def reset_request_counter(vcr_config): - """Reset the request counter in each test""" - vcr_config['before_record_response'].__closure__[0].cell_contents = 0 +@pytest.fixture(autouse=True, scope='function') +def reset_request_counter(vcr_config): + """Reset the request counter in each test""" + def reset_counter(): + nonlocal request_counter + request_counter = 0 + + # Store the reset function in the config for easy access + vcr_config['reset_counter'] = reset_counter + reset_counter()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/test.yml
(1 hunks)README.md
(1 hunks)env.template
(1 hunks)quienesquien/client.py
(1 hunks)quienesquien/person.py
(1 hunks)requirements-test.txt
(1 hunks)requirements.txt
(1 hunks)setup.cfg
(1 hunks)tests/cassettes/test_search.yaml
(1 hunks)tests/cassettes/test_search_not_found.yaml
(1 hunks)tests/cassettes/test_search_with_params.yaml
(1 hunks)tests/conftest.py
(1 hunks)tests/test_client.py
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- env.template
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/cassettes/test_search.yaml
- .github/workflows/test.yml
- tests/cassettes/test_search_not_found.yaml
- requirements-test.txt
🧰 Additional context used
🧠 Learnings (1)
README.md (1)
Learnt from: felipao-mx
PR: cuenca-mx/quienesquien-python#9
File: README.md:20-20
Timestamp: 2025-02-08T01:08:02.602Z
Learning: When reviewing Python library READMEs, ensure that:
1. Examples use explicit values or clear placeholders (e.g., 'https://api.example.com' instead of just 'URL')
2. Both successful and error scenarios are documented with complete code examples
3. All required parameters are clearly explained or demonstrated
🪛 markdownlint-cli2 (0.17.2)
README.md
7-7: Bare URL used
null
(MD034, no-bare-urls)
🔇 Additional comments (16)
requirements.txt (1)
1-1
: Modern Dependency Update: Usinghttpx
The dependencyhttpx==0.28.1
has been added, replacing the older HTTP client libraries. This change aligns with the project's modernization efforts and API integration improvements.README.md (6)
1-7
: Project Title and Badges Consistency
The title is updated to "quienesquien" while the badge URLs (e.g., the test badge on line 3) still reference "quienesquien-python". For consistency, update these URLs if the repository name has officially changed. Additionally, the service description on line 7 uses a bare URL (https://app.q-detect.com/
), which may trigger markdownlint (MD034). Consider formatting it as a proper Markdown link (e.g., using<https://app.q-detect.com/>
or a descriptive link) to improve clarity and compliance.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Bare URL used
null(MD034, no-bare-urls)
9-13
: Clear Installation Instructions
The installation section clearly instructs users on how to install the package usingpip install quienesquien
. This section is concise and effective.
15-25
: Development & Testing Section Approval
The "Development & Testing" section provides clear steps for setting environment variables and running unit tests withpytest
. As a minor note, double-check that the syntaxexport $(<env.template)
works as expected across your target shell environments.
27-34
: Usage Section Clarity
The environment variable configuration is well documented, making it straightforward for users to set up the client before use.
81-91
: Search Parameters Section is Comprehensive
The search parameters are clearly outlined, detailing required and optional fields along with their descriptions. This section meets documentation standards effectively.
94-96
: Response Structure Documentation is Clear
The response structure details are succinct and informative, providing end users with a clear understanding of the fields returned by the client.setup.cfg (2)
5-6
: Great enhancement of test coverage configuration.
Using--cov=quienesquien
with term-missing reporting is helpful.asyncio_mode = auto
also aligns well with async test functions. No immediate issues found.
8-24
: Validate style and coverage settings consistency.
The flake8, isort, mypy, and coverage sections reflect a cohesive approach to code quality. Ensure existing code matches the new quote style rules and that coverage thresholds (if any) are enforced as desired.tests/test_client.py (3)
4-10
: Good negative test coverage for a 'not found' scenario.
Asserting thatresp.success
isFalse
andresp.persons
is empty helps ensure the code handles missing records gracefully. This test is well-structured and focuses on essential outcome checks.
12-18
: Positive test scenario is well-defined.
Verifying that the search returns a non-zeronum_registros
thoroughly checks happy-path functionality.
20-36
: Comprehensive parameterized search test.
Testing multiple search parameters (e.g.,rfc
,curp
,sex
) ensures the client’s ability to handle more advanced filters. The assertions are concise and sufficient.tests/conftest.py (2)
17-21
: LGTM! Good security practice.The masking function effectively prevents sensitive data exposure in test recordings.
24-44
: LGTM! Well-structured VCR configuration.The configuration properly masks sensitive data in headers and query parameters.
quienesquien/client.py (1)
8-12
: LGTM! Clean dataclass implementation.Good use of dataclass for type safety and clear structure.
tests/cassettes/test_search_with_params.yaml (1)
1-109
: LGTM! Well-structured test cassette.The test cassette properly masks sensitive data and provides good coverage of the API interactions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
quienesquien/client.py (1)
43-57
: 🛠️ Refactor suggestionEnhance error handling in _make_request.
The method should handle HTTP errors and raise appropriate exceptions.
Apply this diff to improve error handling:
async def _make_request( self, method: str, url: str, *, headers: Mapping[str, str] | None = None, params: Mapping[str, str | int | None] | None = None, ) -> httpx.Response: """Make an HTTP request using an async client.""" - response = await self._client.request( - method, url, headers=headers, params=params - ) - response.raise_for_status() - return response + try: + response = await self._client.request( + method, url, headers=headers, params=params + ) + response.raise_for_status() + return response + except httpx.HTTPStatusError as e: + raise QuienEsQuienError(f"HTTP error occurred: {e.response.text}") from e + except httpx.RequestError as e: + raise QuienEsQuienError(f"Request failed: {str(e)}") from e
🧹 Nitpick comments (8)
tests/conftest.py (2)
15-17
: Consider scoping the client fixture.The fixture creates a new client for each test. Consider using
scope='module'
orscope='session'
to reuse the client across tests.Apply this diff to scope the fixture:
-@pytest.fixture() +@pytest.fixture(scope='module') async def client() -> AsyncGenerator[Client, None]: yield Client(QEQ_USER, QEQ_CLIENT_ID, QEQ_SECRET_ID)
20-37
: Use Client's base_url for consistency.The base URL is hardcoded in the mock. Consider using
Client.base_url
for consistency.Apply this diff to use Client's base_url:
+from quienesquien import Client def create_mock_response(status_code: int, message: str): """Helper function to create a respx mock fixture.""" @pytest.fixture def _mock(): with respx.mock( - base_url='https://app.q-detect.com', + base_url=Client.base_url, assert_all_called=False, assert_all_mocked=False, ) as respx_mock:tests/test_client.py (3)
17-20
: Extract test data to constants.Consider extracting hardcoded test data to constants for better maintainability.
Apply this diff to extract test data:
+# Test data constants +TEST_NONEXISTENT_NAME = 'Pepito Cuenca' +TEST_MATCH_SCORE = 80 @pytest.mark.vcr async def test_search_not_found(client: Client) -> None: with pytest.raises(PersonNotFoundError): - await client.search('Pepito Cuenca ', 80) + await client.search(TEST_NONEXISTENT_NAME, TEST_MATCH_SCORE)
23-26
: Enhance test assertions and extract test data.The test uses a weak assertion and hardcoded test data.
Apply this diff to improve the test:
+# Test data constants +TEST_KNOWN_NAME = 'Andres Manuel Lopez Obrador' +TEST_MATCH_SCORE = 80 @pytest.mark.vcr async def test_search_by_name(client: Client) -> None: - resp = await client.search('Andres Manuel Lopez Obrador', 80) - assert len(resp) != 0 + resp = await client.search(TEST_KNOWN_NAME, TEST_MATCH_SCORE) + assert len(resp) > 0 + assert any(person.full_name == TEST_KNOWN_NAME for person in resp)
29-39
: Enhance test assertions and extract test data.The test uses a weak assertion and hardcoded test data.
Apply this diff to improve the test:
+# Test data constants +TEST_CURP = 'LOOA531113HTCPBN07' +TEST_MATCH_SCORE = 85 +TEST_BIRTHDAY = dt.date(1953, 11, 13) @pytest.mark.vcr async def test_search_by_curp(client: Client) -> None: resp = await client.search( - match_score=85, - curp='LOOA531113HTCPBN07', + match_score=TEST_MATCH_SCORE, + curp=TEST_CURP, gender=Gender.masculino, - birthday=dt.date(1953, 11, 13), + birthday=TEST_BIRTHDAY, search_type=SearchType.fisica, search_list=(SearchList.PPE, SearchList.ONU), ) - assert len(resp) != 0 + assert len(resp) > 0 + assert any(person.curp == TEST_CURP for person in resp)quienesquien/client.py (1)
28-37
: Consider making base_url configurable.The base URL is hardcoded. Consider making it configurable for testing or different environments.
Apply this diff to make base_url configurable:
@dataclass class Client: - base_url = 'https://app.q-detect.com' + base_url: str = field(default='https://app.q-detect.com') username: str client_id: str secret_key: strtests/cassettes/test_reusable_token.yaml (2)
20-36
: Improve JSON response formatting for readability.
The JSON response in the first interaction is provided as a long inlined string. For improved readability and easier maintenance, consider using a YAML block scalar (using|
or>
) to format the JSON response.
62-125
: Maintain JSON readability in the second interaction.
Just as with the first interaction, the JSON response here is a long inlined string. Converting it to a YAML block scalar would aid in readability and help prevent formatting issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
README.md
(1 hunks)quienesquien/client.py
(1 hunks)quienesquien/enums.py
(1 hunks)requirements-test.txt
(1 hunks)tests/cassettes/test_invalid_token.yaml
(1 hunks)tests/cassettes/test_reusable_token.yaml
(1 hunks)tests/cassettes/test_search_by_curp.yaml
(1 hunks)tests/cassettes/test_search_by_name.yaml
(1 hunks)tests/cassettes/test_search_not_found.yaml
(1 hunks)tests/cassettes/test_search_with_params.yaml
(1 hunks)tests/conftest.py
(1 hunks)tests/test_client.py
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- tests/cassettes/test_search_by_name.yaml
- tests/cassettes/test_search_not_found.yaml
- tests/cassettes/test_search_by_curp.yaml
- tests/cassettes/test_search_with_params.yaml
- requirements-test.txt
- tests/cassettes/test_invalid_token.yaml
🧰 Additional context used
🧠 Learnings (1)
README.md (1)
Learnt from: felipao-mx
PR: cuenca-mx/quienesquien-python#9
File: README.md:20-20
Timestamp: 2025-02-08T01:08:02.602Z
Learning: When reviewing Python library READMEs, ensure that:
1. Examples use explicit values or clear placeholders (e.g., 'https://api.example.com' instead of just 'URL')
2. Both successful and error scenarios are documented with complete code examples
3. All required parameters are clearly explained or demonstrated
🪛 GitHub Actions: test
quienesquien/enums.py
[error] 1-1: ImportError: cannot import name 'StrEnum' from 'enum'. This may be due to using a Python version that does not support StrEnum.
tests/conftest.py
[error] 8-8: ImportError while loading conftest: 'StrEnum' could not be imported, causing the test suite to fail.
🪛 markdownlint-cli2 (0.17.2)
README.md
7-7: Bare URL used
null
(MD034, no-bare-urls)
🪛 Ruff (0.8.2)
quienesquien/client.py
148-148: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
150-150: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
151-151: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
🔇 Additional comments (14)
quienesquien/enums.py (2)
4-6
: LGTM!The enum correctly inherits from
IntEnum
and has appropriate integer values.
64-66
: LGTM!The enum correctly inherits from
StrEnum
and has appropriate string values.tests/conftest.py (2)
10-12
: Add error handling for missing environment variables.Direct access to environment variables will raise KeyError if they're not set.
Apply this diff to add error handling:
-QEQ_USER = os.environ['QEQ_USER'] -QEQ_CLIENT_ID = os.environ['QEQ_CLIENT_ID'] -QEQ_SECRET_ID = os.environ['QEQ_SECRET_ID'] +QEQ_USER = os.environ.get('QEQ_USER') +QEQ_CLIENT_ID = os.environ.get('QEQ_CLIENT_ID') +QEQ_SECRET_ID = os.environ.get('QEQ_SECRET_ID') + +if not all([QEQ_USER, QEQ_CLIENT_ID, QEQ_SECRET_ID]): + raise ValueError( + 'Missing required environment variables. Please set QEQ_USER, ' + 'QEQ_CLIENT_ID, and QEQ_SECRET_ID' + )
59-83
: LGTM!The fixture correctly filters sensitive data and is appropriately scoped to session level.
tests/test_client.py (1)
42-121
: LGTM!The remaining tests provide good coverage of error cases and use appropriate mocking strategies.
quienesquien/client.py (2)
76-86
: LGTM!The parameters are well-documented and use appropriate types and defaults.
153-170
: LGTM!The response handling is robust and uses safe dictionary access methods.
README.md (6)
1-8
: Refine the header description URL formatting.
The header (especially line 7) currently uses a bare URL in the service description. For improved readability and to comply with MD034 (no-bare-urls), consider converting it into an explicit markdown link (e.g.,[Quienesquien service](https://app.q-detect.com/)
).🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Bare URL used
null(MD034, no-bare-urls)
9-13
: Installation section looks clear.
The pip installation instructions are concise and straightforward.
15-25
: Development & Testing instructions are well-articulated.
The steps for setting environment variables (viaenv.template
) and running unit tests withpytest
are clear. It might be useful to note that theenv.template
should document all required variables.
27-34
: Usage instructions are concise.
The guidance on setting up environment variables before using the client is clear and user friendly.
36-73
: Clarify asynchronous context and import requirements in the example.
The example code effectively demonstrates error handling; however, there are two points to address:
- The use of
await client.search(...)
indicates asynchronous execution. It would be clearer if the snippet were wrapped in an async function or referenced with a note (e.g., usingasyncio.run(...)
), so users understand that it must be executed in an async context.- The usage of
dt.date(1953, 11, 13)
requires an import (e.g.,import datetime as dt
). Please add this import or clarify thatdt
is an alias for the datetime module.
Additionally, consider adding a more straightforward “success scenario” example to complement the error handling cases.
75-92
: Search Parameters and Response Structure sections are comprehensive.
The documentation clearly details each parameter and the expected response format, which should help users understand how to construct queries and interpret results.tests/cassettes/test_reusable_token.yaml (1)
126-127
: Version information is correctly specified.
The inclusion of the cassette version (version: 1) at the end is useful for tracking changes in the cassette format.
quienesquien/enums.py
Outdated
class SearchList(StrEnum): | ||
PPE = 'PPE' | ||
PEPINT = 'PEPINT' | ||
OFAC = 'OFAC' | ||
OFACN = 'OFACN' | ||
ONU = 'ONU' | ||
SAT69 = 'SAT69' | ||
SAT69B = 'SAT69B' | ||
SANC = 'SANC' | ||
ATF = 'ATF' | ||
BID = 'BID' | ||
BIS = 'BIS' | ||
BM = 'BM' | ||
BOE = 'BOE' | ||
CBI = 'CBI' | ||
DEA = 'DEA' | ||
DFAT = 'DFAT' | ||
EPA = 'EPA' | ||
FAMI = 'FAMI' | ||
FBI = 'FBI' | ||
FINCEN = 'FINCEN' | ||
FUNC = 'FUNC' | ||
GAFI = 'GAFI' | ||
HMT = 'HMT' | ||
ICE = 'ICE' | ||
INTP = 'INTP' | ||
LMW = 'LMW' | ||
OCDE = 'OCDE' | ||
OSFI = 'OSFI' | ||
FGJ = 'FGJ' | ||
FGR = 'FGR' | ||
FGRCOM = 'FGRCOM' | ||
PIS = 'PIS' | ||
REFIPRE = 'REFIPRE' | ||
SSEU = 'SSEU' | ||
UE = 'UE' | ||
UKMW = 'UKMW' | ||
USMS = 'USMS' | ||
VENC = 'VENC' | ||
CAR = 'CAR' | ||
DPRK = 'DPRK' | ||
DRC = 'DRC' | ||
GB = 'GB' | ||
IRAN = 'IRAN' | ||
IRAQ = 'IRAQ' | ||
LIBY = 'LIBY' | ||
MALI = 'MALI' | ||
ONUAQ = 'ONUAQ' | ||
ONUTAL = 'ONUTAL' | ||
SOMA = 'SOMA' | ||
SOUT = 'SOUT' | ||
SUDA = 'SUDA' | ||
YEME = 'YEME' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the StrEnum import error.
The pipeline failure indicates that StrEnum
is not available in the Python version being used. StrEnum
was introduced in Python 3.11.
Apply this diff to fix the import error:
-from enum import IntEnum, StrEnum
+from enum import IntEnum
+try:
+ from enum import StrEnum
+except ImportError:
+ from enum import Enum
+ class StrEnum(str, Enum):
+ """Compatibility class for Python < 3.11."""
+ pass
📝 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.
class SearchList(StrEnum): | |
PPE = 'PPE' | |
PEPINT = 'PEPINT' | |
OFAC = 'OFAC' | |
OFACN = 'OFACN' | |
ONU = 'ONU' | |
SAT69 = 'SAT69' | |
SAT69B = 'SAT69B' | |
SANC = 'SANC' | |
ATF = 'ATF' | |
BID = 'BID' | |
BIS = 'BIS' | |
BM = 'BM' | |
BOE = 'BOE' | |
CBI = 'CBI' | |
DEA = 'DEA' | |
DFAT = 'DFAT' | |
EPA = 'EPA' | |
FAMI = 'FAMI' | |
FBI = 'FBI' | |
FINCEN = 'FINCEN' | |
FUNC = 'FUNC' | |
GAFI = 'GAFI' | |
HMT = 'HMT' | |
ICE = 'ICE' | |
INTP = 'INTP' | |
LMW = 'LMW' | |
OCDE = 'OCDE' | |
OSFI = 'OSFI' | |
FGJ = 'FGJ' | |
FGR = 'FGR' | |
FGRCOM = 'FGRCOM' | |
PIS = 'PIS' | |
REFIPRE = 'REFIPRE' | |
SSEU = 'SSEU' | |
UE = 'UE' | |
UKMW = 'UKMW' | |
USMS = 'USMS' | |
VENC = 'VENC' | |
CAR = 'CAR' | |
DPRK = 'DPRK' | |
DRC = 'DRC' | |
GB = 'GB' | |
IRAN = 'IRAN' | |
IRAQ = 'IRAQ' | |
LIBY = 'LIBY' | |
MALI = 'MALI' | |
ONUAQ = 'ONUAQ' | |
ONUTAL = 'ONUTAL' | |
SOMA = 'SOMA' | |
SOUT = 'SOUT' | |
SUDA = 'SUDA' | |
YEME = 'YEME' | |
from enum import IntEnum | |
try: | |
from enum import StrEnum | |
except ImportError: | |
from enum import Enum | |
class StrEnum(str, Enum): | |
"""Compatibility class for Python < 3.11.""" | |
pass | |
class SearchList(StrEnum): | |
PPE = 'PPE' | |
PEPINT = 'PEPINT' | |
OFAC = 'OFAC' | |
OFACN = 'OFACN' | |
ONU = 'ONU' | |
SAT69 = 'SAT69' | |
SAT69B = 'SAT69B' | |
SANC = 'SANC' | |
ATF = 'ATF' | |
BID = 'BID' | |
BIS = 'BIS' | |
BM = 'BM' | |
BOE = 'BOE' | |
CBI = 'CBI' | |
DEA = 'DEA' | |
DFAT = 'DFAT' | |
EPA = 'EPA' | |
FAMI = 'FAMI' | |
FBI = 'FBI' | |
FINCEN = 'FINCEN' | |
FUNC = 'FUNC' | |
GAFI = 'GAFI' | |
HMT = 'HMT' | |
ICE = 'ICE' | |
INTP = 'INTP' | |
LMW = 'LMW' | |
OCDE = 'OCDE' | |
OSFI = 'OSFI' | |
FGJ = 'FGJ' | |
FGR = 'FGR' | |
FGRCOM = 'FGRCOM' | |
PIS = 'PIS' | |
REFIPRE = 'REFIPRE' | |
SSEU = 'SSEU' | |
UE = 'UE' | |
UKMW = 'UKMW' | |
USMS = 'USMS' | |
VENC = 'VENC' | |
CAR = 'CAR' | |
DPRK = 'DPRK' | |
DRC = 'DRC' | |
GB = 'GB' | |
IRAN = 'IRAN' | |
IRAQ = 'IRAQ' | |
LIBY = 'LIBY' | |
MALI = 'MALI' | |
ONUAQ = 'ONUAQ' | |
ONUTAL = 'ONUTAL' | |
SOMA = 'SOMA' | |
SOUT = 'SOUT' | |
SUDA = 'SUDA' | |
YEME = 'YEME' |
quienesquien/client.py
Outdated
except httpx.HTTPStatusError as exc: | ||
if exc.response.status_code == 401: | ||
self._invalidate_auth_token() | ||
raise InvalidTokenError(exc.response) | ||
if exc.response.status_code == 403: | ||
raise InsufficientBalanceError(exc.response) | ||
raise QuienEsQuienError(exc.response) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix exception chaining.
Exception chaining is missing in the error handling code.
Apply this diff to fix exception chaining:
except httpx.HTTPStatusError as exc:
if exc.response.status_code == 401:
self._invalidate_auth_token()
- raise InvalidTokenError(exc.response)
+ raise InvalidTokenError(exc.response) from exc
if exc.response.status_code == 403:
- raise InsufficientBalanceError(exc.response)
+ raise InsufficientBalanceError(exc.response) from exc
- raise QuienEsQuienError(exc.response)
+ raise QuienEsQuienError(exc.response) from exc
📝 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.
except httpx.HTTPStatusError as exc: | |
if exc.response.status_code == 401: | |
self._invalidate_auth_token() | |
raise InvalidTokenError(exc.response) | |
if exc.response.status_code == 403: | |
raise InsufficientBalanceError(exc.response) | |
raise QuienEsQuienError(exc.response) | |
except httpx.HTTPStatusError as exc: | |
if exc.response.status_code == 401: | |
self._invalidate_auth_token() | |
raise InvalidTokenError(exc.response) from exc | |
if exc.response.status_code == 403: | |
raise InsufficientBalanceError(exc.response) from exc | |
raise QuienEsQuienError(exc.response) from exc |
🧰 Tools
🪛 Ruff (0.8.2)
148-148: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
150-150: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
151-151: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
tests/conftest.py (1)
8-10
:⚠️ Potential issueAdd error handling for environment variables.
Direct access to environment variables can raise KeyError if they're not set. Add proper error handling with descriptive messages.
Apply this diff to add error handling:
-QEQ_USER = os.environ['QEQ_USER'] -QEQ_CLIENT_ID = os.environ['QEQ_CLIENT_ID'] -QEQ_SECRET_ID = os.environ['QEQ_SECRET_ID'] +QEQ_USER = os.environ.get('QEQ_USER') +QEQ_CLIENT_ID = os.environ.get('QEQ_CLIENT_ID') +QEQ_SECRET_ID = os.environ.get('QEQ_SECRET_ID') + +if not all([QEQ_USER, QEQ_CLIENT_ID, QEQ_SECRET_ID]): + raise ValueError( + 'Missing required environment variables. Please set QEQ_USER, ' + 'QEQ_CLIENT_ID, and QEQ_SECRET_ID' + )quienesquien/client.py (1)
147-153
:⚠️ Potential issueFix exception chaining in error handling.
Exception chaining is missing in the error handling code.
Apply this diff to fix exception chaining:
except httpx.HTTPStatusError as exc: if exc.response.status_code == 401: self._invalidate_auth_token() - raise InvalidTokenError(exc.response) + raise InvalidTokenError(exc.response) from exc if exc.response.status_code == 403: - raise InsufficientBalanceError(exc.response) + raise InsufficientBalanceError(exc.response) from exc raise QuienEsQuienError(exc.response) from exc🧰 Tools
🪛 Ruff (0.8.2)
150-150: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
152-152: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
🧹 Nitpick comments (4)
quienesquien/client.py (1)
28-37
: Consider making base_url configurable.The base_url is hardcoded. Consider making it configurable through environment variables or constructor parameters to support different environments (e.g., staging, testing).
Apply this diff to make base_url configurable:
@dataclass class Client: - base_url = 'https://app.q-detect.com' + base_url: str = field(default='https://app.q-detect.com') username: str client_id: str secret_key: strtests/test_client.py (2)
18-64
: Consider using anonymized test data.The tests use real person data which might be sensitive. Consider using anonymized test data to protect privacy.
Example of anonymized test data:
@pytest.mark.vcr async def test_search_by_name(client: Client) -> None: resp = await client.search('John Smith', 80) # Use anonymized name assert len(resp) != 0
178-186
: Add vcr marker for consistency.For consistency with other tests, consider adding the
@pytest.mark.vcr
decorator.Apply this diff:
+@pytest.mark.vcr async def test_invalid_search_criteria(client: Client) -> None: with pytest.raises(InvalidSearchCriteriaError): await client.search(
tests/cassettes/test_insufficient_balance.yaml (1)
47-77
: Insufficient Balance ScenarioThe second interaction simulates an error due to insufficient balance. The JSON error message (including escaped characters) is provided as expected. Confirm that the receiving test properly decodes the escaped characters (e.g. in "b\xFAsqueda") so that the error message is verified correctly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
quienesquien/client.py
(1 hunks)quienesquien/enums.py
(1 hunks)tests/cassettes/test_block_account.yaml
(1 hunks)tests/cassettes/test_insufficient_balance.yaml
(1 hunks)tests/cassettes/test_internal_server_error.yaml
(1 hunks)tests/cassettes/test_invalid_plan.yaml
(1 hunks)tests/cassettes/test_invalid_token.yaml
(1 hunks)tests/cassettes/test_reusable_token.yaml
(1 hunks)tests/cassettes/test_search_by_curp.yaml
(1 hunks)tests/cassettes/test_search_by_name.yaml
(1 hunks)tests/cassettes/test_search_not_found.yaml
(1 hunks)tests/cassettes/test_search_with_params.yaml
(1 hunks)tests/cassettes/test_token_error_response.yaml
(1 hunks)tests/cassettes/test_unknown_error.yaml
(1 hunks)tests/conftest.py
(1 hunks)tests/test_client.py
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- quienesquien/enums.py
- tests/cassettes/test_search_by_name.yaml
- tests/cassettes/test_invalid_token.yaml
- tests/cassettes/test_search_by_curp.yaml
- tests/cassettes/test_search_with_params.yaml
🧰 Additional context used
🪛 Ruff (0.8.2)
quienesquien/client.py
150-150: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
152-152: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
🔇 Additional comments (20)
tests/conftest.py (1)
18-38
: LGTM! Well-structured VCR configuration.The fixture correctly:
- Uses session scope for performance
- Filters sensitive data from responses
- Filters sensitive headers and query parameters
quienesquien/client.py (2)
39-41
: LGTM! Clear and focused implementation.The method correctly invalidates the token with a clear purpose.
58-76
: LGTM! Well-implemented token management.The method correctly:
- Caches and reuses tokens
- Handles errors with proper exception chaining
- Uses appropriate headers
tests/test_client.py (1)
66-176
: LGTM! Comprehensive error handling tests.The tests correctly:
- Mock API responses using respx_mock
- Cover various error scenarios
- Validate error responses and token invalidation
tests/cassettes/test_token_error_response.yaml (1)
1-31
: LGTM! Well-structured VCR interaction.The interaction correctly:
- Filters sensitive data (Authorization, client_id)
- Simulates error response with appropriate status code
- Includes required headers
tests/cassettes/test_unknown_error.yaml (2)
1-46
: Review of Token Endpoint InteractionThe cassette defines the token retrieval interaction correctly. The request and response sections are well structured and include all required headers and expected values. Make sure that the fixed values (e.g. the static Date header) are acceptable for tests or are appropriately mocked.
47-75
: Review of "Unknown error" InteractionThe second interaction correctly simulates an API response indicating an unknown error. The request is properly formed with query parameters and the response contains a JSON body with a clear error message. Verify that the consuming test (e.g. in
tests/test_client.py
) correctly interprets the HTTP 200 response as an error condition based on the response body.tests/cassettes/test_internal_server_error.yaml (2)
1-46
: Token Endpoint Interaction ConsistencyThis interaction mimics the token retrieval scenario with the expected static header values and response content. The structure is consistent with other cassette files.
47-75
: Internal Server Error SimulationThe second interaction accurately simulates a scenario where the API returns a 500 status code along with an appropriate JSON error message. This setup should allow tests to verify error handling logic for internal server errors.
tests/cassettes/test_insufficient_balance.yaml (1)
1-46
: Token Retrieval Interaction for Insufficient Balance TestsThe cassette’s first interaction (token retrieval) is correctly structured with the expected headers and response values. Minor note: Verify that the static Date header (line 32) matches the intended test setup.
tests/cassettes/test_block_account.yaml (1)
1-46
: 🛠️ Refactor suggestion
⚠️ Potential issueYAML Structure: Token Interaction
The cassette starts by defining a token request. However, unlike other cassette files where each interaction is encapsulated in a single dictionary (combining both request and response), here the token request (lines 2–18) is immediately followed by a separate list item starting with
- response:
(lines 19–46). This splitting does not follow the standard structure used in the other cassette files.Suggested diff to merge the token interaction into one item:
-interactions: - - request: - body: '' - headers: - Authorization: - - DUMMY_AUTHORIZATION - accept: - - '*/*' - accept-encoding: - - identity - connection: - - keep-alive - host: - - app.q-detect.com - user-agent: - - python-httpx/0.28.1 - method: GET - uri: https://app.q-detect.com/api/token?client_id=DUMMY_CLIENT_ID - - response: - body: - string: DUMMY_RESPONSE_TOKEN - headers: - Access-Control-Allow-Origin: - - '*' - Cache-Control: - - no-cache, private - Connection: - - keep-alive - Content-Type: - - text/html; charset=UTF-8 - Date: - - Thu, 20 Feb 2025 15:49:53 GMT - Server: - - nginx/1.18.0 (Ubuntu) - Set-Cookie: DUMMY_SET_COOKIE - Transfer-Encoding: - - chunked - X-Content-Type-Options: - - nosniff - X-Frame-Options: - - SAMEORIGIN - X-XSS-Protection: - - 1; mode=block - status: - code: 200 - message: OK +interactions: + - request: + body: '' + headers: + Authorization: + - DUMMY_AUTHORIZATION + accept: + - '*/*' + accept-encoding: + - identity + connection: + - keep-alive + host: + - app.q-detect.com + user-agent: + - python-httpx/0.28.1 + method: GET + uri: https://app.q-detect.com/api/token?client_id=DUMMY_CLIENT_ID + response: + body: + string: DUMMY_RESPONSE_TOKEN + headers: + Access-Control-Allow-Origin: + - '*' + Cache-Control: + - no-cache, private + Connection: + - keep-alive + Content-Type: + - text/html; charset=UTF-8 + Date: + - Thu, 20 Feb 2025 15:49:53 GMT + Server: + - nginx/1.18.0 (Ubuntu) + Set-Cookie: DUMMY_SET_COOKIE + Transfer-Encoding: + - chunked + X-Content-Type-Options: + - nosniff + X-Frame-Options: + - SAMEORIGIN + X-XSS-Protection: + - 1; mode=block + status: + code: 200 + message: OKLikely an incorrect or invalid review comment.
tests/cassettes/test_invalid_plan.yaml (2)
1-46
: Valid Plan Token InteractionThe first interaction is structured correctly. The request and its corresponding response (for retrieving the token) are combined within one list item, which is consistent with cassette conventions elsewhere in the repository.
47-76
: Invalid Plan Error InteractionThe second interaction simulates an expired consultation plan with an expected error message. Note that the API returns a 200 HTTP status code despite the error status in the response body. Verify that this behavior is intended and that the test logic in
tests/test_client.py
handles such cases as designed.tests/cassettes/test_search_not_found.yaml (3)
1-47
: Token Retrieval Interaction is Configured Correctly
The token request interaction includes all the expected headers and query parameters, and returns a dummy token with the proper HTTP status and header configuration.
47-91
: Search Interaction for “Not Found” Response Looks Appropriate
The search request interaction returns a JSON body indicating no matches (i.e.,"success": false
with an appropriate status message). Ensure that your client logic properly translates this response into aPersonNotFoundError
(or the equivalent behavior) during testing.
92-93
: YAML Versioning is Correct
The version field is set to 1, which matches the configuration requirements.tests/cassettes/test_reusable_token.yaml (4)
1-46
: Token Request Interaction Setup is Solid
The token request interaction is defined with all required headers and parameters. The response correctly simulates a successful token retrieval with a dummy token.
47-107
: Search Interaction for “Andres Manuel López Obrador” is Detailed and Comprehensive
This interaction returns a JSON payload with multiple detailed fields, including Unicode escapes for special characters. Please ensure that your asynchronous client handles multi-record responses correctly and that the JSON parsing properly decodes Unicode characters such as in “López Obrador”.
107-171
: Search Interaction for “Marcelo Luis Ebrard Casaubón” is Well-Configured
The cassette for Marcelo Luis Ebrard Casaubón provides a rich JSON response with multiple person records. Confirm that your client’s response handling logic correctly processes multiple entries and decodes URI-encoded characters (e.g.,Casaub%C3%B3n
) into their proper form. This will help ensure robustness when managing diverse and complex data returned by the API.
172-172
: YAML Versioning is Consistent
The version key is correctly specified as 1.
- request: | ||
body: '' | ||
headers: | ||
Authorization: | ||
- DUMMY_AUTHORIZATION | ||
accept: | ||
- '*/*' | ||
accept-encoding: | ||
- gzip, deflate | ||
connection: | ||
- keep-alive | ||
host: | ||
- app.q-detect.com | ||
user-agent: | ||
- python-httpx/0.28.1 | ||
method: GET | ||
uri: https://app.q-detect.com/api/find?client_id=DUMMY_CLIENT_ID&name=Pepito+Cuenca&percent=80&username=DUMMY_USERNAME | ||
response: | ||
body: | ||
string: "{\"success\":false,\"status\":\"El token proporcionado para realizar | ||
esta acci\xF3n es inv\xE1lido\"}" | ||
headers: | ||
Content-Length: | ||
- '91' | ||
Content-Type: | ||
- application/json | ||
Set-Cookie: DUMMY_SET_COOKIE | ||
status: | ||
code: 401 | ||
message: Unauthorized | ||
version: 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
YAML Structure: Block Account Error Interaction
A similar issue is present in the second interaction for the block account scenario (lines 47–77). The request and response are provided as separate list items rather than as a single combined interaction. Refactoring the structure as suggested above will improve consistency and reduce potential parsing errors with your testing framework.
quienesquien/client.py
Outdated
except httpx.HTTPStatusError as exc: | ||
if exc.response.status_code == 401: | ||
self._invalidate_auth_token() | ||
raise InvalidTokenError(exc.response) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
para estos ifs puedes utilizar match https://docs.python.org/es/dev/tutorial/controlflow.html#tut-match
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
De acuerdo, realice el cambio
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
quienesquien/client.py (2)
21-21
: Consider making base_url configurable.The hardcoded base URL could make testing difficult. Consider making it configurable through the constructor while keeping the current value as default.
@dataclass class Client: - base_url = 'https://app.q-detect.com' + base_url: str = 'https://app.q-detect.com'
72-72
: Document match_score parameter constraints.The comment should explain why 60 is the default value and any constraints on valid values.
- match_score: int = 60, # Default 60, applied even if no value provided + match_score: int = 60, # Default 60 as recommended by API docs. Valid range: 0-100
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
quienesquien/client.py
(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
quienesquien/client.py
142-142: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
144-144: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
except httpx.HTTPStatusError as exc: | ||
match exc.response.status_code: | ||
case 401: | ||
self._invalidate_auth_token() | ||
raise InvalidTokenError(exc.response) | ||
case 403: | ||
raise InsufficientBalanceError(exc.response) | ||
case _: | ||
raise QuienEsQuienError(exc.response) from exc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix exception chaining in error handling.
Exception chaining is missing in some error cases, which could hide the original cause.
except httpx.HTTPStatusError as exc:
match exc.response.status_code:
case 401:
self._invalidate_auth_token()
- raise InvalidTokenError(exc.response)
+ raise InvalidTokenError(exc.response) from exc
case 403:
- raise InsufficientBalanceError(exc.response)
+ raise InsufficientBalanceError(exc.response) from exc
case _:
raise QuienEsQuienError(exc.response) from exc
📝 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.
except httpx.HTTPStatusError as exc: | |
match exc.response.status_code: | |
case 401: | |
self._invalidate_auth_token() | |
raise InvalidTokenError(exc.response) | |
case 403: | |
raise InsufficientBalanceError(exc.response) | |
case _: | |
raise QuienEsQuienError(exc.response) from exc | |
except httpx.HTTPStatusError as exc: | |
match exc.response.status_code: | |
case 401: | |
self._invalidate_auth_token() | |
raise InvalidTokenError(exc.response) from exc | |
case 403: | |
raise InsufficientBalanceError(exc.response) from exc | |
case _: | |
raise QuienEsQuienError(exc.response) from exc |
🧰 Tools
🪛 Ruff (0.8.2)
142-142: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
144-144: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
async def _make_request( | ||
self, | ||
method: str, | ||
url: str, | ||
*, | ||
headers: Mapping[str, str] | None = None, | ||
params: Mapping[str, str | int | None] | None = None, | ||
) -> httpx.Response: | ||
"""Make an HTTP request using an async client.""" | ||
response = await self._client.request( | ||
method, url, headers=headers, params=params | ||
) | ||
response.raise_for_status() | ||
return response |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance error handling in _make_request.
The method should handle timeouts and include exception chaining.
async def _make_request(
self,
method: str,
url: str,
*,
headers: Mapping[str, str] | None = None,
params: Mapping[str, str | int | None] | None = None,
) -> httpx.Response:
"""Make an HTTP request using an async client."""
+ timeout = httpx.Timeout(10.0, connect=5.0)
try:
response = await self._client.request(
- method, url, headers=headers, params=params
+ method, url, headers=headers, params=params, timeout=timeout
)
response.raise_for_status()
return response
+ except httpx.TimeoutException as e:
+ raise QuienEsQuienError("Request timed out") from e
+ except httpx.HTTPStatusError as e:
+ raise QuienEsQuienError(f"HTTP error occurred: {e.response.text}") from e
+ except httpx.RequestError as e:
+ raise QuienEsQuienError(f"Request failed: {str(e)}") from e
📝 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.
async def _make_request( | |
self, | |
method: str, | |
url: str, | |
*, | |
headers: Mapping[str, str] | None = None, | |
params: Mapping[str, str | int | None] | None = None, | |
) -> httpx.Response: | |
"""Make an HTTP request using an async client.""" | |
response = await self._client.request( | |
method, url, headers=headers, params=params | |
) | |
response.raise_for_status() | |
return response | |
async def _make_request( | |
self, | |
method: str, | |
url: str, | |
*, | |
headers: Mapping[str, str] | None = None, | |
params: Mapping[str, str | int | None] | None = None, | |
) -> httpx.Response: | |
"""Make an HTTP request using an async client.""" | |
timeout = httpx.Timeout(10.0, connect=5.0) | |
try: | |
response = await self._client.request( | |
method, url, headers=headers, params=params, timeout=timeout | |
) | |
response.raise_for_status() | |
return response | |
except httpx.TimeoutException as e: | |
raise QuienEsQuienError("Request timed out") from e | |
except httpx.HTTPStatusError as e: | |
raise QuienEsQuienError(f"HTTP error occurred: {e.response.text}") from e | |
except httpx.RequestError as e: | |
raise QuienEsQuienError(f"Request failed: {str(e)}") from e |
Description
This pull request updates the client to integrate with the new API version. It includes type annotations, corresponding tests, and GitHub Actions for automated testing and release processes.
Changes
Summary by CodeRabbit
New Features
Documentation
CI/CD & Build Improvements
Enhancements
Version 1.0.0 marks this update, elevating user experience and developer efficiency.