Skip to content
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

Merged
merged 41 commits into from
Feb 20, 2025
Merged

Feature/new api integration #9

merged 41 commits into from
Feb 20, 2025

Conversation

gmorales96
Copy link

@gmorales96 gmorales96 commented Feb 7, 2025

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

  • Updated the client to use the new API version.
  • Added type annotations for improved type safety.
  • Implemented corresponding unit and integration tests.
  • Added GitHub Actions workflows for testing and releasing.

Summary by CodeRabbit

  • New Features

    • Introduced asynchronous search capabilities with improved error handling for a more responsive experience.
    • Added support for multiple search parameters including CURP and gender.
    • Implemented a versioning system for the module.
    • Added new enumeration classes for search types and gender.
    • Introduced custom exception classes for better error management.
    • Added automated workflows for release and testing to streamline deployments.
  • Documentation

    • Revamped documentation now provides clear installation, usage, and development guidelines.
  • CI/CD & Build Improvements

    • Enhanced CI/CD workflows for automated testing and deployment.
    • New testing configurations and dependencies for improved code quality assurance.
  • Enhancements

    • Modernized build processes and dependency management deliver improved performance and reliability.

Version 1.0.0 marks this update, elevating user experience and developer efficiency.

Copy link

coderabbitai bot commented Feb 7, 2025

Walkthrough

The repository now includes two new GitHub workflows: one automates package publishing to PyPI on release events (using .github/workflows/release.yml), and the other manages linting, testing across multiple Python versions, and coverage reporting (using .github/workflows/test.yml). The Makefile has been updated to use Python 3.13 and now features modified and new targets for installation, testing, formatting, and releasing. Documentation in README.md has been revised to reflect the new project name and include detailed sections for installation, usage, and development. Within the codebase, the public interface in __init__.py has been redefined, the Client class has been refactored to use asynchronous methods for token fetching and searching, and the Person model now extends Pydantic’s BaseModel with explicit fields. Additionally, dependencies in requirements files and configuration in setup files have been adjusted, new test configurations and VCR cassette files have been added, and new enumerations and custom exception classes have been introduced.

✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@gmorales96 gmorales96 self-assigned this Feb 7, 2025
Copy link

@coderabbitai coderabbitai bot left a 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 suggestion

Replace 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 using importlib.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 the coincidencia field.

The coincidencia field has been added to PERSON_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

📥 Commits

Reviewing files that changed from the base of the PR and between 695910c and d1d5848.

📒 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:

  1. The latest compatible requests version
  2. 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.

Copy link

codecov bot commented Feb 7, 2025

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 ☂️

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between d1d5848 and e0d3d98.

📒 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.

@gmorales96 gmorales96 force-pushed the feature/new-api-integration branch from e0d3d98 to daeec2d Compare February 8, 2025 00:43
Copy link

@coderabbitai coderabbitai bot left a 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 suggestion

Enhance error handling and validate token response.

The method should handle API-specific errors and validate the token format.


32-40: ⚠️ Potential issue

Add URL encoding for search parameters.

Direct string interpolation in URLs can lead to encoding issues or potential security vulnerabilities.


42-65: 🛠️ Refactor suggestion

Enhance 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

📥 Commits

Reviewing files that changed from the base of the PR and between e0d3d98 and daeec2d.

📒 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.

Copy link
Member

@felipao-mx felipao-mx left a 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 ?

Copy link

@coderabbitai coderabbitai bot left a 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 issue

Add 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 uses await on asynchronous calls (lines 47 and 67), it would be beneficial to wrap the snippet inside an asynchronous context (e.g., using async def main(): with asyncio.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 the Person class would improve clarity and help document usage.


6-69: Confirm field types and leverage domain-specific types if possible.
All fields are typed as str | None or int | 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 add from __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

📥 Commits

Reviewing files that changed from the base of the PR and between daeec2d and bbfdcea.

📒 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: Using httpx
The dependency httpx==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 using pip 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 with pytest. As a minor note, double-check that the syntax export $(<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 that resp.success is False and resp.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-zero num_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.

Copy link

@coderabbitai coderabbitai bot left a 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 suggestion

Enhance 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' or scope='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: str
tests/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

📥 Commits

Reviewing files that changed from the base of the PR and between 854c595 and 4e82be9.

📒 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 (via env.template) and running unit tests with pytest are clear. It might be useful to note that the env.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., using asyncio.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 that dt 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.

Comment on lines 9 to 61
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'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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'

Comment on lines 145 to 151
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)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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)

Copy link

@coderabbitai coderabbitai bot left a 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 issue

Add 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 issue

Fix 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 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)

🧹 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: str
tests/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 Scenario

The 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

📥 Commits

Reviewing files that changed from the base of the PR and between 4e82be9 and 29cc903.

📒 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 Interaction

The 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" Interaction

The 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 Consistency

This 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 Simulation

The 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 Tests

The 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 issue

YAML 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: OK

Likely an incorrect or invalid review comment.

tests/cassettes/test_invalid_plan.yaml (2)

1-46: Valid Plan Token Interaction

The 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 Interaction

The 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 a PersonNotFoundError (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.

Comment on lines +47 to +77
- 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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

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.

except httpx.HTTPStatusError as exc:
if exc.response.status_code == 401:
self._invalidate_auth_token()
raise InvalidTokenError(exc.response)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

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

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between df8dcc6 and 88a371f.

📒 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)

Comment on lines +138 to +146
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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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)

Comment on lines +34 to +47
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
Copy link

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.

Suggested change
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

@pachCode pachCode merged commit 23dc02d into master Feb 20, 2025
10 checks passed
@pachCode pachCode deleted the feature/new-api-integration branch February 20, 2025 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants