Skip to content

Conversation

@gmorales96
Copy link
Contributor

@gmorales96 gmorales96 commented Oct 11, 2025

Summary by CodeRabbit

  • Breaking Changes

    • Minimum Python raised to 3.10; validation library upgraded and HTTP client replaced—these may require code changes for compatibility.
  • Refactor

    • Internal HTTP and data models modernized to newer typing and serialization semantics.
  • Chores

    • CI workflows, packaging metadata, and dependency pins updated to current tool versions and Python targets.
  • Tests

    • Tests updated to align with new serialization and type behaviors.

@coderabbitai
Copy link

coderabbitai bot commented Oct 11, 2025

Walkthrough

Workflows and CI updated to newer GitHub Action major versions (checkout@v4, setup-python@v5), Python matrices raised (now including 3.10–3.13) and the release workflow now triggers on published releases; Codecov action/token added. Project minimum Python requirement raised to >=3.10 (setup.py, classifiers, Makefile). Dependencies moved from requests to httpx and Pydantic upgraded from v1 to v2 (requirements and setup.py). HTTP client rewritten from requests.Session to httpx.Client with BasicAuth and adjusted response handling. Typing modernized to PEP 604 unions and built-in generics; Pydantic models migrated to v2 APIs (Annotated, Field, model_dump). Tests updated to use model_dump and minor assertion style changes. Package version set to '1.0.0'.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning In addition to migrating Pydantic and replacing requests with httpx, this PR includes broad CI and tooling updates such as workflow version bumps, Python version upgrades in workflows and the Makefile, and release trigger changes that are unrelated to the linked issue’s objectives. Separate the pipeline and tooling modifications into a dedicated PR or scope this change to only the Pydantic and httpx migration to keep the review focused on the linked issue objectives.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “Update deps” is partially related as it signals dependency updates but is too generic to convey the primary intent of migrating to Pydantic v2 and replacing requests with httpx.
Linked Issues Check ✅ Passed The changes update the Pydantic dependency to v2 across setup and requirements, refactor models and validation to use Pydantic v2 syntax and model_dump, and fully replace requests with httpx in the HTTP client and dependencies, satisfying all coding objectives from issue #116.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-deps

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a2a1147 and 0fd54df.

📒 Files selected for processing (1)
  • facturapi/version.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

⚙️ CodeRabbit configuration file

**/*.py: Enforce Relative Imports for Internal Modules

Ensure that any imports referencing internal modules use relative paths. However, if modules reside in the main module directories (for example /src or /library_or_app_name) —and relative imports are not feasible—absolute imports are acceptable. Additionally, if a module is located outside the main module structure (for example, in /tests or /scripts at a similar level), absolute imports are also valid.

Examples and Guidelines:

  1. If a module is in the same folder or a subfolder of the current file, use relative imports. For instance: from .some_module import SomeClass
  2. If the module is located under /src or /library_or_app_name and cannot be imported relatively, absolute imports are allowed (e.g., from library_or_app_name.utilities import helper_method).
  3. If a module is outside the main module directories (for example, in /tests, /scripts, or any similarly placed directory), absolute imports are valid.
  4. External (third-party) libraries should be imported absolutely (e.g., import requests).

**/*.py:
Rule: Enforce Snake Case in Python Backend

  1. New or Modified Code: Use snake_case for all variables, functions, methods, and class attributes.
  2. Exceptions (Pydantic models for API responses):
    • Primary fields must be snake_case.
    • If older clients expect camelCase, create a computed or alias field that references the snake_case field.
    • Mark any camelCase fields as deprecated or transitional.

Examples

Invalid:

class CardConfiguration(BaseModel):
    title: str
    subTitle: str  # ❌ Modified or new field in camelCase

Valid:

class CardConfiguration(BaseModel):
    title: str
    subtitle: str  # ✅ snake_case for new/modified field

    @computed_field
    def subTitle(self) -> str:  # camelCase allowed only for compatibility
        return self.subtitle

Any direct use of camelCase in new or updated code outside of these exceptions should be flagged.

`*...

Files:

  • facturapi/version.py
🔇 Additional comments (1)
facturapi/version.py (1)

1-1: Confirm packaging metadata reflects new version. setup.py contains no literal version= entry; ensure it dynamically sources __version__ = '1.0.0' or update your setup.cfg/pyproject.toml accordingly.


Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Oct 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (4f39663) to head (0fd54df).
⚠️ Report is 29 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #117   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        15           
  Lines          466       470    +4     
=========================================
+ Hits           466       470    +4     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
facturapi/http/client.py 100.00% <100.00%> (ø)
facturapi/resources/base.py 100.00% <100.00%> (ø)
facturapi/resources/customers.py 100.00% <100.00%> (ø)
facturapi/resources/invoices.py 100.00% <100.00%> (ø)
facturapi/resources/resources.py 100.00% <100.00%> (ø)
facturapi/types/exc.py 100.00% <100.00%> (ø)
facturapi/types/general.py 100.00% <100.00%> (ø)
facturapi/types/queries.py 100.00% <100.00%> (ø)
facturapi/version.py 100.00% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a2eb83c...0fd54df. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)
.github/workflows/release.yml (1)

11-15: Update action versions and Python version for consistency.

This workflow still uses outdated action versions and Python 3.8, while other workflows in this PR were updated to:

  • actions/checkout@v4 (currently using @master)
  • actions/setup-python@v5 (currently using @v2.2.2)
  • Python 3.13 (currently using 3.8)

This inconsistency may lead to different build environments between testing and release.

Apply this diff to update the workflow:

-      - uses: actions/checkout@master
-      - name: Set up Python 3.8
-        uses: actions/setup-python@v2.2.2
+      - uses: actions/checkout@v4
+      - name: Set up Python 3.13
+        uses: actions/setup-python@v5
         with:
-          python-version: 3.8
+          python-version: 3.13
.github/workflows/test.yml (1)

25-33: Fix YAML indentation for pytest steps.

The list items under steps: are aligned with the parent key, so this file no longer parses (steps: expects its items indented). Reindent the step entries by two spaces so the workflow loads correctly.

Apply this diff:

-    - uses: actions/checkout@v4
-    - name: Set up Python ${{ matrix.python-version }}
-      uses: actions/setup-python@v5
-      with:
-        python-version: ${{ matrix.python-version }}
-    - name: Install dependencies
-      run: make install-test
-    - name: Run tests
-      run: pytest -x
+      - uses: actions/checkout@v4
+      - name: Set up Python ${{ matrix.python-version }}
+        uses: actions/setup-python@v5
+        with:
+          python-version: ${{ matrix.python-version }}
+      - name: Install dependencies
+        run: make install-test
+      - name: Run tests
+        run: pytest -x
facturapi/resources/customers.py (1)

106-106: Replace Pydantic v1 .dict() with v2 .model_dump().

The .dict() method is deprecated in Pydantic v2 and has been replaced with .model_dump(). Since this PR upgrades to Pydantic v2 (as noted in the AI summary), these calls will fail at runtime.

Apply this diff:

-        cleaned_data = data.dict(exclude_unset=True, exclude_none=True)
+        cleaned_data = data.model_dump(exclude_unset=True, exclude_none=True)

The same pattern appears on both lines 106 and 121.

Based on learnings about Pydantic v2 migration.

Also applies to: 121-121

facturapi/http/client.py (1)

33-44: Resource leak: httpx.Client is not closed.

The httpx.Client instance is created but never explicitly closed, which can lead to resource leaks (open connections, file descriptors). Unlike requests.Session, httpx clients should be properly managed.

Consider one of these solutions:

Solution 1 (Recommended): Add a close method and use context manager pattern

+    def close(self) -> None:
+        """Close the HTTP client and release resources."""
+        self.client.close()
+
+    def __enter__(self) -> 'Client':
+        return self
+
+    def __exit__(self, *args) -> None:
+        self.close()

Then users can use:

with client:
    # make requests
    pass

Solution 2: Document that users should call client.client.close() when done

At minimum, add this to the class docstring to alert users of the responsibility.

Based on learnings about httpx resource management.

facturapi/resources/invoices.py (1)

177-177: Replace Pydantic v1 .dict() with v2 .model_dump().

The .dict() method must be replaced with .model_dump() for Pydantic v2 compatibility. This is a breaking change that will cause runtime failures.

Apply this diff:

-        cleaned_data = data.dict(exclude_unset=True, exclude_none=True)
+        cleaned_data = data.model_dump(exclude_unset=True, exclude_none=True)

Based on learnings about Pydantic v2 migration.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a2eb83c and 8218b9e.

📒 Files selected for processing (17)
  • .github/workflows/auto_merge.yml (1 hunks)
  • .github/workflows/docs.yml (1 hunks)
  • .github/workflows/release.yml (1 hunks)
  • .github/workflows/test.yml (3 hunks)
  • Makefile (1 hunks)
  • facturapi/http/client.py (7 hunks)
  • facturapi/resources/base.py (6 hunks)
  • facturapi/resources/customers.py (4 hunks)
  • facturapi/resources/invoices.py (7 hunks)
  • facturapi/resources/resources.py (1 hunks)
  • facturapi/types/exc.py (2 hunks)
  • facturapi/types/general.py (2 hunks)
  • facturapi/types/queries.py (3 hunks)
  • requirements-test.txt (1 hunks)
  • requirements.txt (1 hunks)
  • setup.py (1 hunks)
  • tests/resources/test_invoices.py (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

⚙️ CodeRabbit configuration file

**/*.py: Enforce Relative Imports for Internal Modules

Ensure that any imports referencing internal modules use relative paths. However, if modules reside in the main module directories (for example /src or /library_or_app_name) —and relative imports are not feasible—absolute imports are acceptable. Additionally, if a module is located outside the main module structure (for example, in /tests or /scripts at a similar level), absolute imports are also valid.

Examples and Guidelines:

  1. If a module is in the same folder or a subfolder of the current file, use relative imports. For instance: from .some_module import SomeClass
  2. If the module is located under /src or /library_or_app_name and cannot be imported relatively, absolute imports are allowed (e.g., from library_or_app_name.utilities import helper_method).
  3. If a module is outside the main module directories (for example, in /tests, /scripts, or any similarly placed directory), absolute imports are valid.
  4. External (third-party) libraries should be imported absolutely (e.g., import requests).

**/*.py:
Rule: Enforce Snake Case in Python Backend

  1. New or Modified Code: Use snake_case for all variables, functions, methods, and class attributes.
  2. Exceptions (Pydantic models for API responses):
    • Primary fields must be snake_case.
    • If older clients expect camelCase, create a computed or alias field that references the snake_case field.
    • Mark any camelCase fields as deprecated or transitional.

Examples

Invalid:

class CardConfiguration(BaseModel):
    title: str
    subTitle: str  # ❌ Modified or new field in camelCase

Valid:

class CardConfiguration(BaseModel):
    title: str
    subtitle: str  # ✅ snake_case for new/modified field

    @computed_field
    def subTitle(self) -> str:  # camelCase allowed only for compatibility
        return self.subtitle

Any direct use of camelCase in new or updated code outside of these exceptions should be flagged.

`*...

Files:

  • tests/resources/test_invoices.py
  • facturapi/types/exc.py
  • facturapi/resources/customers.py
  • facturapi/http/client.py
  • setup.py
  • facturapi/resources/base.py
  • facturapi/types/queries.py
  • facturapi/resources/resources.py
  • facturapi/types/general.py
  • facturapi/resources/invoices.py
🧬 Code graph analysis (4)
facturapi/resources/customers.py (2)
facturapi/types/enums.py (1)
  • TaxSystemType (99-122)
facturapi/types/general.py (1)
  • CustomerAddress (6-30)
facturapi/resources/base.py (1)
tests/types/test_types.py (1)
  • to_dict (17-18)
facturapi/resources/resources.py (1)
facturapi/resources/base.py (1)
  • Retrievable (69-105)
facturapi/resources/invoices.py (3)
facturapi/types/general.py (3)
  • ProductBasicInfo (60-65)
  • ItemPart (41-49)
  • Namespace (52-57)
facturapi/resources/customers.py (1)
  • CustomerRequest (18-38)
facturapi/types/enums.py (4)
  • PaymentForm (65-89)
  • PaymentMethod (92-96)
  • InvoiceUse (36-62)
  • InvoiceRelation (12-23)
🔇 Additional comments (16)
.github/workflows/docs.yml (1)

11-15: LGTM! Action versions and Python version updated consistently.

The workflow updates align with the project-wide Python 3.13 upgrade and use current action versions.

Makefile (2)

3-3: LGTM! Python version updated to 3.13.

Consistent with the project-wide Python upgrade.


6-6: LGTM! Black target version updated to match Python 3.13.

The formatting target aligns with the updated Python version.

.github/workflows/auto_merge.yml (1)

10-10: LGTM! Checkout action updated to v4.

Consistent with action version updates across other workflows.

.github/workflows/release.yml (1)

3-5: LGTM! Improved release trigger.

Triggering on release.published instead of push is more appropriate for release workflows and prevents accidental publishes.

requirements-test.txt (1)

1-7: LGTM! Test tooling upgraded to current versions.

All test dependencies have been updated to their latest major versions. These updates align with the Python 3.13 upgrade and should provide improved functionality and compatibility.

Note that major version bumps (pytest 6→8, mypy 0.8→1, etc.) may introduce breaking changes, so ensure the test suite passes with these new versions.

facturapi/resources/resources.py (2)

2-2: LGTM! Import cleaned up for built-in generics.

Removing the Dict import is correct since the code now uses the built-in dict type annotation (Python 3.9+).


7-7: LGTM! Type annotation modernized to built-in generic.

The update from Dict[str, Retrievable] to dict[str, Retrievable] uses Python 3.9+ built-in generics, consistent with the project's Python 3.10+ requirement.

facturapi/types/exc.py (3)

2-2: LGTM! Import updated for built-in generics.

Removed Dict import as the code now uses built-in dict type annotations.


6-6: LGTM! Improved class body style.

Using pass instead of ellipsis (...) is the conventional style for empty class bodies in Python.


19-19: LGTM! Type annotation modernized to built-in generic.

The update from Dict[str, Any] to dict[str, Any] follows Python 3.9+ conventions, consistent with the project's upgrade.

facturapi/resources/customers.py (1)

8-8: LGTM! Typing modernization to PEP 604 syntax.

The migration from Optional[T] to T | None and the removal of Optional from imports aligns with modern Python 3.10+ typing standards and is consistent with the broader codebase changes.

Also applies to: 37-37, 56-61, 92-93

facturapi/resources/base.py (1)

9-9: LGTM! Consistent typing modernization.

The migration to built-in generic types (list, dict) and PEP 604 union syntax (T | None) is correctly applied throughout the base resource classes. The explicit -> None return type annotations improve type safety.

Also applies to: 34-34, 43-43, 48-48, 65-65, 93-93, 259-259

facturapi/http/client.py (2)

62-63: LGTM! Type hints modernized correctly.

The method signatures now use PEP 604 union syntax and built-in generic types, which is consistent with the Python 3.10+ target for this project.

Also applies to: 67-67, 71-71, 75-75, 83-86


107-115: LGTM – httpx API usage is correct. All of json=, response.is_success, httpx.BasicAuth, and the request() signature are supported in httpx 0.28.1.

facturapi/resources/invoices.py (1)

49-57: LGTM! Typing modernization applied consistently.

All type annotations have been correctly updated to use:

  • PEP 604 union syntax (T | None)
  • Built-in generic types (list[T], dict)
  • Proper nullable defaults with = None

The changes maintain backward compatibility while improving type safety.

Also applies to: 94-109, 157-164, 199-199

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/types/test_queries.py (1)

2-2: Import ValidationError from pydantic (v2-compatible).

pydantic.error_wrappers is v1-era; v2 exposes ValidationError at top-level.

-from pydantic.error_wrappers import ValidationError
+from pydantic import ValidationError
🧹 Nitpick comments (4)
facturapi/resources/invoices.py (3)

49-50: Avoid nullable quantity/discount; make them non-optional with defaults.

Let’s prevent None from creeping in and being silently dropped by exclude_none. These fields behave as required with sensible defaults.

-    quantity: int | None = 1
-    discount: float | None = 0
+    quantity: int = 1
+    discount: float = 0

33-36: Fix docstring inconsistencies (types/names).

Several attribute docs don’t match the code:

  • quantity is int (not str)
  • customs_keys name (not custom_keys)
  • parts uses ItemPart (not ItemParts)
  • namespaces is a single Namespace (not list[Namespace])
-        quantity (str): Number of items of this type. Defaults
+        quantity (int): Number of items of this type. Defaults
           to `1`.
-        custom_keys (list[str]): list of custom product keys.
+        customs_keys (list[str]): list of customs product keys.
           Optional.
-        parts (list[ItemParts]): If the concept includes parts.
+        parts (list[ItemPart]): If the concept includes parts.
           Optional.
-        namespaces (list[Namespace]): If `addenda` or an item complement
+        namespaces (Namespace): If `addenda` or an item complement
           is given, the special namespaces of the XML code. Optional.

Also applies to: 39-45, 89-90


215-218: Update exception type to httpx.

Docs still mention requests.RequestException after the migration to httpx.

-            requests.RequestException: If the API request fails.
+            httpx.HTTPError: If the API request fails.
facturapi/types/queries.py (1)

4-4: Prefer ConfigDict for model_config.

Minor modernization to align with Pydantic v2 style.

-from pydantic import BaseModel, Field
+from pydantic import BaseModel, Field, ConfigDict
@@
-    model_config = {"extra": "forbid"}
+    model_config = ConfigDict(extra='forbid')

Also applies to: 59-59

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8218b9e and f241c42.

📒 Files selected for processing (4)
  • facturapi/resources/customers.py (6 hunks)
  • facturapi/resources/invoices.py (8 hunks)
  • facturapi/types/queries.py (3 hunks)
  • tests/types/test_queries.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • facturapi/resources/customers.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

⚙️ CodeRabbit configuration file

**/*.py: Enforce Relative Imports for Internal Modules

Ensure that any imports referencing internal modules use relative paths. However, if modules reside in the main module directories (for example /src or /library_or_app_name) —and relative imports are not feasible—absolute imports are acceptable. Additionally, if a module is located outside the main module structure (for example, in /tests or /scripts at a similar level), absolute imports are also valid.

Examples and Guidelines:

  1. If a module is in the same folder or a subfolder of the current file, use relative imports. For instance: from .some_module import SomeClass
  2. If the module is located under /src or /library_or_app_name and cannot be imported relatively, absolute imports are allowed (e.g., from library_or_app_name.utilities import helper_method).
  3. If a module is outside the main module directories (for example, in /tests, /scripts, or any similarly placed directory), absolute imports are valid.
  4. External (third-party) libraries should be imported absolutely (e.g., import requests).

**/*.py:
Rule: Enforce Snake Case in Python Backend

  1. New or Modified Code: Use snake_case for all variables, functions, methods, and class attributes.
  2. Exceptions (Pydantic models for API responses):
    • Primary fields must be snake_case.
    • If older clients expect camelCase, create a computed or alias field that references the snake_case field.
    • Mark any camelCase fields as deprecated or transitional.

Examples

Invalid:

class CardConfiguration(BaseModel):
    title: str
    subTitle: str  # ❌ Modified or new field in camelCase

Valid:

class CardConfiguration(BaseModel):
    title: str
    subtitle: str  # ✅ snake_case for new/modified field

    @computed_field
    def subTitle(self) -> str:  # camelCase allowed only for compatibility
        return self.subtitle

Any direct use of camelCase in new or updated code outside of these exceptions should be flagged.

`*...

Files:

  • tests/types/test_queries.py
  • facturapi/types/queries.py
  • facturapi/resources/invoices.py
🧬 Code graph analysis (1)
facturapi/resources/invoices.py (3)
facturapi/types/general.py (3)
  • ProductBasicInfo (60-65)
  • ItemPart (41-49)
  • Namespace (52-57)
facturapi/resources/customers.py (1)
  • CustomerRequest (18-38)
facturapi/types/enums.py (4)
  • PaymentForm (65-89)
  • PaymentMethod (92-96)
  • InvoiceUse (36-62)
  • InvoiceRelation (12-23)
🔇 Additional comments (3)
facturapi/resources/invoices.py (1)

177-178: LGTM: Pydantic v2 serialization via model_dump.

Correct use of model_dump with exclude_unset/none in create().

tests/types/test_queries.py (1)

9-11: LGTM: model_dump usage in assertions.

Matches the v2 serialization approach with exclude_unset/none.

facturapi/types/queries.py (1)

9-11: LGTM: Correct page constraints and defaults.

  • Page uses ge=MIN_PAGE (keeps page=1 valid).
  • Defaults are plain ints (no Annotated callables). Resolves prior blockers.

Also applies to: 55-57

rogelioLpz
rogelioLpz previously approved these changes Oct 15, 2025
@rogelioLpz rogelioLpz merged commit 46f146d into main Oct 15, 2025
11 checks passed
@rogelioLpz rogelioLpz deleted the update-deps branch October 15, 2025 21:26
@rogelioLpz rogelioLpz restored the update-deps branch October 15, 2025 21:28
@rogelioLpz rogelioLpz deleted the update-deps branch October 15, 2025 21:28
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.

Migrar de Pydantic v1 a v2 y reemplazar requests por httpx

3 participants