-
Notifications
You must be signed in to change notification settings - Fork 2
Update deps #117
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
Update deps #117
Conversation
This reverts commit 47fba50.
WalkthroughWorkflows 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)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.py⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 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
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
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 -xfacturapi/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.Clientinstance is created but never explicitly closed, which can lead to resource leaks (open connections, file descriptors). Unlikerequests.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 passSolution 2: Document that users should call
client.client.close()when doneAt 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
📒 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 ModulesEnsure 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:
- 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
- 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).
- If a module is outside the main module directories (for example, in /tests, /scripts, or any similarly placed directory), absolute imports are valid.
- External (third-party) libraries should be imported absolutely (e.g., import requests).
**/*.py:
Rule: Enforce Snake Case in Python Backend
- New or Modified Code: Use snake_case for all variables, functions, methods, and class attributes.
- 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 camelCaseValid:
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.subtitleAny direct use of camelCase in new or updated code outside of these exceptions should be flagged.
`*...
Files:
tests/resources/test_invoices.pyfacturapi/types/exc.pyfacturapi/resources/customers.pyfacturapi/http/client.pysetup.pyfacturapi/resources/base.pyfacturapi/types/queries.pyfacturapi/resources/resources.pyfacturapi/types/general.pyfacturapi/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.publishedinstead ofpushis 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
Dictimport is correct since the code now uses the built-indicttype annotation (Python 3.9+).
7-7: LGTM! Type annotation modernized to built-in generic.The update from
Dict[str, Retrievable]todict[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
Dictimport as the code now uses built-indicttype annotations.
6-6: LGTM! Improved class body style.Using
passinstead 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]todict[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]toT | Noneand the removal ofOptionalfrom 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-> Nonereturn 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 ofjson=,response.is_success,httpx.BasicAuth, and therequest()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
= NoneThe changes maintain backward compatibility while improving type safety.
Also applies to: 94-109, 157-164, 199-199
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 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
📒 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 ModulesEnsure 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:
- 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
- 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).
- If a module is outside the main module directories (for example, in /tests, /scripts, or any similarly placed directory), absolute imports are valid.
- External (third-party) libraries should be imported absolutely (e.g., import requests).
**/*.py:
Rule: Enforce Snake Case in Python Backend
- New or Modified Code: Use snake_case for all variables, functions, methods, and class attributes.
- 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 camelCaseValid:
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.subtitleAny direct use of camelCase in new or updated code outside of these exceptions should be flagged.
`*...
Files:
tests/types/test_queries.pyfacturapi/types/queries.pyfacturapi/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
Summary by CodeRabbit
Breaking Changes
Refactor
Chores
Tests