chore/pyo3 0.27 #83
chore/pyo3 0.27 #83
Conversation
pyo3 to v0.27 pyo3-stub-gen to 2bec9c3 Signed-off-by: FITAHIANA Nomeniavo Joe <24nomeniavo@gmail.com>
|
Caution Review failedThe pull request is closed. WalkthroughVersion bump and dependency updates; many .pyi formatting/type-hint whitespace edits; adds IntoPyException trait and blanket impl; changes into_response DEBUG handling to default true; wraps several extraction returns in Changes
Sequence Diagram(s)sequenceDiagram
participant Rust as Rust code
participant Exceptions as crate::exceptions
participant Py as Python layer
rect rgb(240,248,255)
Note over Rust,Exceptions: Exception conversion (new)
Rust->>Exceptions: result.into_py_exception()
Exceptions-->>Rust: PyResult<T> (maps Err -> PyException)
Rust->>Py: return PyResult
end
sequenceDiagram
participant Request as Request handler
participant IntoResp as into_response.rs
participant Logger as Error display
Request->>IntoResp: produce error value
alt DEBUG unset or unparsable (defaults true)
IntoResp->>Logger: value.display(py)
else DEBUG explicitly "false"
Note right of IntoResp: skip display
end
IntoResp->>Request: return Response with error info
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
README.md (1)
12-12: Add alt text for the version badge imageThe new version badge
<img>lacksalt, which markdownlint (MD045) flags and which slightly hurts accessibility. A brief alt text (or empty alt if you consider it decorative) would address this.Example:
-<a href='https://github.com/j03-dev/oxapy/#'><img src='https://img.shields.io/badge/version-0.7.4-%23b7410e'/></a> +<a href='https://github.com/j03-dev/oxapy/#'><img src='https://img.shields.io/badge/version-0.7.4-%23b7410e' alt='OxAPY version 0.7.4'/></a>src/exceptions.rs (1)
13-21:IntoPyExceptionprovides a clean, reusable Result→PyErr bridgeThe public
IntoPyException<T>trait with its blanket impl forResult<T, E: ToString>is a straightforward way to convert common Rust errors (e.g., fromjsonschema,Mutex::lock, IO) intoPyExceptions, and it matches how the rest of the crate is using it.Since this is now part of the public
exceptionsAPI, consider adding a brief doc comment explaining that it’s intended for turning ordinaryResult<T, E>values intoPyResult<T>for Python interop.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
Cargo.toml(2 hunks)README.md(1 hunks)oxapy/__init__.pyi(44 hunks)oxapy/exceptions/__init__.pyi(7 hunks)oxapy/jwt/__init__.pyi(4 hunks)oxapy/serializer/__init__.pyi(17 hunks)oxapy/templating/__init__.pyi(2 hunks)pyproject.toml(1 hunks)src/bin/stub_gen.rs(0 hunks)src/exceptions.rs(1 hunks)src/into_response.rs(2 hunks)src/json.rs(2 hunks)src/jwt.rs(1 hunks)src/lib.rs(2 hunks)src/serializer/mod.rs(1 hunks)
💤 Files with no reviewable changes (1)
- src/bin/stub_gen.rs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-22T14:44:51.678Z
Learnt from: j03-dev
Repo: j03-dev/oxapy PR: 82
File: src/routing.rs:412-417
Timestamp: 2025-11-22T14:44:51.678Z
Learning: In oxapy's Router (src/routing.rs), the `middleware()` method implements a layered architecture: it adds the middleware to the current layer, then creates a new empty layer via `self.layers.push(Layer::default())`. This is intentional - subsequent routes registered after the middleware call go into the new layer without inheriting the middleware from the previous layer. Each layer maintains its own routes and middlewares independently. When a request is processed, the system searches through layers to find a matching route and applies only that layer's middlewares.
Applied to files:
oxapy/__init__.pyi
🧬 Code graph analysis (3)
src/lib.rs (1)
src/exceptions.rs (1)
exceptions(138-148)
src/into_response.rs (1)
src/exceptions.rs (1)
exceptions(138-148)
oxapy/serializer/__init__.pyi (1)
oxapy/exceptions/__init__.pyi (1)
ClientError(20-31)
🪛 markdownlint-cli2 (0.18.1)
README.md
12-12: Images should have alternate text (alt text)
(MD045, no-alt-text)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: musllinux (ubuntu-22.04, aarch64)
- GitHub Check: linux (ubuntu-22.04, aarch64)
- GitHub Check: macos (macos-14, aarch64)
- GitHub Check: linux (ubuntu-22.04, s390x)
- GitHub Check: linux (ubuntu-22.04, armv7)
- GitHub Check: linux (ubuntu-22.04, ppc64le)
- GitHub Check: linux (ubuntu-22.04, x86_64)
- GitHub Check: linux (ubuntu-22.04, x86)
- GitHub Check: musllinux (ubuntu-22.04, x86)
- GitHub Check: windows (windows-latest, x86)
- GitHub Check: windows (windows-latest, x64)
- GitHub Check: musllinux (ubuntu-22.04, armv7)
- GitHub Check: musllinux (ubuntu-22.04, x86_64)
🔇 Additional comments (7)
src/json.rs (1)
20-21: ExplicitOk(...)wrapping keeps PyResult semantics clearWrapping
serialized_data.extract(py)?/deserialized_data.extract(py)?inOk(...)matches thePyResult<_>signature of the closures and keeps error propagation unchanged. Looks good.Also applies to: 29-30
pyproject.toml (1)
19-21: Confirmpython-source="."matches your package layoutSetting
python-source="."makes maturin treat the repo root as the Python source directory, which is fine if youroxapypackage lives there. If your Python code is undersrc/or elsewhere, this may need to be adjusted.Please verify that
maturin build(or your CI build) still discovers and packagesoxapyas expected with this setting.src/serializer/mod.rs (1)
247-269: Single-instancedatabranch now aligns withPyResultreturn typeReturning
Ok(repr.extract()?)in the non-manypath matches thePyResult<Py<PyAny>>signature and is consistent with how errors are propagated elsewhere in this getter.Cargo.toml (1)
3-3: Dependency and crate version bumps look consistent with the PR goalsThe move to
oxapy0.7.4 and the pyo3 / pyo3-async-runtimes / pyo3-stub-gen updates align with the stated objective of tracking pyo3 0.27.Please make sure your CI (or local matrix) exercises all supported Python versions with these new versions (especially pyo3 0.27 and pyo3-async-runtimes 0.27) to catch any upstream breaking changes early.
Also applies to: 16-18
src/lib.rs (1)
24-27: ReusingIntoPyExceptionfromexceptionscentralizes error conversionImporting
crate::exceptions::IntoPyException(withpyo3::prelude::*) avoids re-defining the trait here and keeps conversions likectrlc::set_handler(...).into_py_exception()?consistent across the crate.Also applies to: 48-49
src/into_response.rs (1)
4-7: DEBUG handling andIntoPyExceptionusage match the intended behaviorImporting
IntoPyExceptionhere ties in with theto_response!macro’s use ofvalue.try_into().into_py_exception(), keeping Rust→Python error conversion consistent.For
From<PyErr> for Response, the updated DEBUG logic now prints the Python exception by default and only suppressesvalue.display(py)whenDEBUG=false, which aligns with the “display the error as default” goal while still returning a 500 with a JSONdetailfield.Also applies to: 89-95
oxapy/serializer/__init__.pyi (1)
186-186: No breaking changes identified. The review comment can be resolved.The addition of the
typarameter toField.__new__does not break existing code because:
All Field subclasses (CharField, EmailField, DateField, etc.) have their own
__new__signatures that do not include thetyparameter. They start withrequiredas the first optional parameter.Only subclass instantiation is used in the codebase (
serializer.EmailField(),serializer.CharField(min_length=8), etc.). No directField()instantiation was found.The Field docstring explicitly states: "You usually use one of the subclasses (
CharField,EmailField,IntegerField, etc.) rather than instantiating this directly."The stub-only nature of this package (backed by PyO3) means subclasses completely override the parent
__new__method, so the Field signature change has no impact on runtime usage.
Signed-off-by: FITAHIANA Nomeniavo Joe <24nomeniavo@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/bin/stub_gen.rs (1)
8-23: Consider checking the subprocess exit status and avoiding unused output captureRight now
Command::new("find")...output()?;only fails on IO/spawn errors and ignores the actualfind/sedexit status, and it also captures stdout/stderr you later discard.Two small improvements:
- Avoid capturing output when you don’t use it:
- .output()?; + .status()?;
- (Optional but recommended) Check
status.success()and surface a meaningful error if the post‑processing fails, so brokenfind/sedinvocations don’t silently leave stubs unmodified.Also keep in mind this implicitly relies on GNU
find/sedsemantics (especiallysed -i), so this binary is effectively “Linux-only” unless your CI/dev story already assumes those tools.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
oxapy/__init__.pyi(44 hunks)oxapy/exceptions/__init__.pyi(7 hunks)oxapy/jwt/__init__.pyi(4 hunks)oxapy/serializer/__init__.pyi(17 hunks)src/bin/stub_gen.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- oxapy/serializer/init.pyi
🚧 Files skipped from review as they are similar to previous changes (2)
- oxapy/exceptions/init.pyi
- oxapy/init.pyi
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: windows (windows-latest, x64)
- GitHub Check: linux (ubuntu-22.04, armv7)
- GitHub Check: linux (ubuntu-22.04, x86)
- GitHub Check: linux (ubuntu-22.04, s390x)
- GitHub Check: linux (ubuntu-22.04, x86_64)
- GitHub Check: linux (ubuntu-22.04, ppc64le)
- GitHub Check: musllinux (ubuntu-22.04, aarch64)
- GitHub Check: windows (windows-latest, x86)
- GitHub Check: musllinux (ubuntu-22.04, x86_64)
- GitHub Check: musllinux (ubuntu-22.04, x86)
- GitHub Check: musllinux (ubuntu-22.04, armv7)
- GitHub Check: linux (ubuntu-22.04, aarch64)
🔇 Additional comments (3)
src/bin/stub_gen.rs (1)
3-3: Import position and aliasing look fineUsing
pyo3_stub_gen::Resultas the main return type is consistent with the rest of this binary; no issues here.oxapy/jwt/__init__.pyi (2)
12-67: Jwt API signatures look consistent with documented behaviorThe constructor and
generate_token/verify_tokenannotations match the described usage and return types; the formatting-only tweaks here don’t introduce any typing or API issues. No changes requested.
103-130: JWT exception__new__annotations now correctly return the exception typeThe
__new__methods forJwtDecodingError,JwtEncodingError,JwtError,JwtInvalidAlgorithm, andJwtInvalidClaimnow return their respective exception classes instead of a tuple, which aligns the stubs with runtime behavior and resolves the earlier critical typing problem. Looks good.
Summary by CodeRabbit
Chores
Style
Documentation
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.