Chore/pyo3 25 - #68
Conversation
…ndle non manualy is best away
WalkthroughRefactors the HTTP server’s request/response pipeline, replaces PyObject with Py across bindings, adds extensive stub-generation via pyo3_stub_gen, introduces RequestBuilder, enriches Response/Cors/Catcher/JWT/Router APIs, adds multipart handling changes, expands exceptions, updates packaging (rlib, new stub_gen bin), and revises tooling and tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Hyper as Hyper Server
participant RB as RequestBuilder
participant Router
participant MW as MiddlewareChain
participant PyH as Python Handler
participant Conv as into_response
participant Post as Post-Process
participant Net as Network Reply
Client->>Hyper: HTTP Request
Hyper->>RB: Build Request (headers/body/session/multipart)
RB-->>Hyper: Request
Hyper->>Router: Find route & params
alt Middlewares exist
Router->>MW: Build chain
MW->>PyH: Call(request, **params)
else No middleware
Router->>PyH: Call(request, **params)
end
PyH-->>Conv: Python result (value/status)
Conv-->>Hyper: Response
Hyper->>Post: Apply catchers/session cookies/CORS
Post-->>Net: Hyper Response
Net-->>Client: HTTP Response
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
src/templating/minijinja.rs (2)
35-35: Compile error: io::Error not converted to PyErr.
?onread_to_stringwon’t convertio::ErrortoPyErr. Use your IntoPyException helper.- let content = std::fs::read_to_string(&path)?; + let content = std::fs::read_to_string(&path).into_py_exception()?;
31-34: Path handling is not cross‑platform and may compute wrong template names.Splitting on "/" and skipping one segment breaks on Windows and for various globs. Use std::path to derive a stable name (example below keeps just the file name; adapt if you need subdirs).
- let full_path = path.to_str().unwrap().to_string(); - let name = full_path.split("/").skip(1); - name.collect::<Vec<_>>().join("/") + use std::path::Path; + Path::new(&path) + .file_name() + .and_then(|s| s.to_str()) + .ok_or_else(|| PyValueError::new_err("Non-UTF8 file name"))? + .to_string()Optional: if you need the path relative to a templates root, compute a base directory (without glob parts) and use
path.strip_prefix(base)then normalize\to/.src/jwt.rs (1)
116-121: Token generation ignores configured algorithm (always HS256).
Header::default()sets HS256;verify_tokenusesself.algorithm. Non‑HS256 configs will fail verification.- let token = jsonwebtoken::encode( - &Header::default(), + let header = Header::new(self.algorithm); + let token = jsonwebtoken::encode( + &header, &claims, &EncodingKey::from_secret(self.secret.as_bytes()), )src/session.rs (1)
398-398: Avoid unwrap on session.last_accessed too.- *session.last_accessed.lock().unwrap() = SystemTime::now() + *session.last_accessed.lock().into_py_exception()? = SystemTime::now()src/into_response.rs (1)
22-34: Bytes return path is missing; bytes from Python will be JSON-serialized or failHandlers returning
bytesshould produce an octet-stream (or respectResponse::new’s bytes path). Add aPy<PyBytes>conversion and try it before the genericPy<PyAny>.use pyo3::{prelude::*, types::PyAny, Py}; +use pyo3::types::PyBytes; @@ impl TryFrom<Py<PyAny>> for Response { @@ } +impl TryFrom<Py<PyBytes>> for Response { + type Error = Error; + fn try_from(val: Py<PyBytes>) -> Result<Self, Self::Error> { + Python::with_gil(|py| { + let mut headers = HeaderMap::new(); + headers.insert(CONTENT_TYPE, "application/octet-stream".parse()?); + Ok(Response { + status: Status::OK, + headers, + body: val.as_bytes(py).to_vec().into(), + }) + }) + } +} @@ pub fn convert_to_response(result: Py<PyAny>, py: Python<'_>) -> PyResult<Response> { to_response!( result, py, Response, Status, (String, Status), + Py<PyBytes>, (Py<PyAny>, Status), String, Py<PyAny> ) }Also applies to: 50-62, 122-135
src/response.rs (3)
126-131: Avoid panic in headers() getter; return PyResult and propagate UTF‑8 errors.
HeaderValue::to_str()can fail; currentunwrap()will panic. Also your docstring says this can raise, so make the signature fallible.- #[getter] - fn headers(&self) -> Vec<(&str, &str)> { - self.headers - .iter() - .map(|(k, v)| (k.as_str(), v.to_str().unwrap())) - .collect() - } + #[getter] + fn headers(&self) -> PyResult<Vec<(String, String)>> { + self.headers + .iter() + .map(|(k, v)| { + Ok(( + k.as_str().to_string(), + v.to_str().into_py_exception()?.to_string(), + )) + }) + .collect() + }
147-152: Header parsing uses unwrap(); convert to exceptions instead.Invalid header names/values from Python will crash the process. Return a
PyResultand map errors.- pub fn insert_header(&mut self, key: &str, value: String) { - self.headers.insert( - HeaderName::from_bytes(key.as_bytes()).unwrap(), - value.parse().unwrap(), - ); - } + pub fn insert_header(&mut self, key: &str, value: String) -> PyResult<()> { + let name = HeaderName::from_bytes(key.as_bytes()).into_py_exception()?; + let value = value.parse().into_py_exception()?; + self.headers.insert(name, value); + Ok(()) + } @@ - pub fn append_header(&mut self, key: &str, value: String) { - self.headers.append( - HeaderName::from_bytes(key.as_bytes()).unwrap(), - value.parse().unwrap(), - ); - } + pub fn append_header(&mut self, key: &str, value: String) -> PyResult<()> { + let name = HeaderName::from_bytes(key.as_bytes()).into_py_exception()?; + let value = value.parse().into_py_exception()?; + self.headers.append(name, value); + Ok(()) + }Note: Adjust stubs to reflect
-> Nonewith potential exceptions (Pythonically fine).Also applies to: 173-178
238-250: Redirect::new should not unwrap LOCATION; return an error on invalid URL.Unvalidated
locationcan makeparse()fail and panic.- #[new] - fn new(location: String) -> (Redirect, Response) { + #[new] + fn new(location: String) -> PyResult<(Redirect, Response)> { let mut headers = HeaderMap::new(); - headers.insert(CONTENT_TYPE, "text/html".parse().unwrap()); - headers.insert(LOCATION, location.parse().unwrap()); - ( + headers.insert(CONTENT_TYPE, "text/html".parse().into_py_exception()?); + headers.insert(LOCATION, location.parse().into_py_exception()?); + Ok(( Self, Response { status: Status::MOVED_PERMANENTLY, body: Bytes::new(), headers, }, - ) + )) }src/routing.rs (2)
461-469: Undefined behavior: lifetime extension with unsafe transmute in Router::find.
transmute(route)to'staticis unsound; it can outlive theRwLockguard and the request URI buffer. This can cause UB across tasks.Recommended shape: return owned data (params and handler), not references.
- pub(crate) fn find<'l>(&'l self, method: &str, uri: &'l str) -> Option<MatchRoute<'l>> { + pub(crate) fn find_owned(&self, method: &str, uri: &str) + -> Option<(Vec<(String, String)>, Arc<Py<PyAny>>, Router)> { let path = uri.split('?').next().unwrap_or(uri); - let routes_guard = self.routes.read().ok()?; - let router = routes_guard.get(method)?; - let route = router.at(path).ok()?; - let route: MatchRoute = unsafe { transmute(route) }; - Some(route) + let routes_guard = self.routes.read().ok()?; + let router = routes_guard.get(method)?; + let matched = router.at(path).ok()?; + let params = matched + .params + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(); + let handler = matched.value.handler.clone(); + Some((params, handler, self.clone())) }Follow-up diffs provided in request.rs/lib.rs to consume this safely.
503-518: static_file path traversal; restrict to directory and set plain‑text for 404.
"{directory}/{path}"allows../to escape the base. Also 404 currently returns JSON (default content-type). Harden the Python shim.- c_str!( - r#" -def static_file(request, path): - file_path = f"{directory}/{path}" - try: - with open(file_path, "rb") as f: content = f.read() - content_type, _ = mimetypes.guess_type(file_path) - return Response(content, content_type = content_type or "application/octet-stream") - except FileNotFoundError: - return Response("File not found", Status.NOT_FOUND) -"# - ), + c_str!(r#" +def static_file(request, path): + base = Path(directory).resolve() + target = (base / path).resolve() + # Prevent directory traversal + if not str(target).startswith(str(base)): + return Response("Forbidden", Status.FORBIDDEN, "text/plain") + try: + with open(target, "rb") as f: + content = f.read() + content_type, _ = mimetypes.guess_type(str(target)) + return Response(content, content_type=content_type or "application/octet-stream") + except FileNotFoundError: + return Response("File not found", Status.NOT_FOUND, "text/plain") +"#),src/lib.rs (1)
58-65: ProcessRequest should hold owned params/handler instead of MatchRoute<'static>.This pairs with the routing/request change and removes UB.
-struct ProcessRequest { +struct ProcessRequest { request: Arc<Request>, router: Option<Arc<Router>>, - match_route: Option<MatchRoute<'static>>, + params: Option<Vec<(String, String)>>, + handler: Option<Arc<Py<PyAny>>>, tx: Sender<Response>, cors: Option<Arc<Cors>>, catchers: Option<Arc<HashMap<Status, Py<PyAny>>>>, }
🧹 Nitpick comments (40)
README.md (1)
12-12: Add alt text to images (accessibility + markdownlint MD045).Provide alt attributes for badges/images.
-<a href='https://github.com/j03-dev/oxapy/#'><img src='https://img.shields.io/badge/version-0.6.2-%23b7410e'/></a> +<a href='https://github.com/j03-dev/oxapy/#'><img src='https://img.shields.io/badge/version-0.6.2-%23b7410e' alt='OxAPY version 0.6.2'/></a> - <a href="https://github.com/j03-dev/bench"><img src="https://bench-n9zz.onrender.com/bench"/></a> + <a href="https://github.com/j03-dev/bench"><img src="https://bench-n9zz.onrender.com/bench" alt="OxAPY benchmark chart"/></a>Also applies to: 24-24
pyproject.toml (1)
2-2: Python floor bumped to 3.10—confirm intent and advertise supported versions.
- Dropping 3.8/3.9 is a breaking packaging change; ensure this is intentional.
- Add explicit Python version classifiers to match requires-python.
classifiers = [ "Programming Language :: Rust", "Programming Language :: Python :: Implementation :: CPython", "Programming Language :: Python :: Implementation :: PyPy", + "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", + "Programming Language :: Python :: 3.13", ]Also applies to: 7-7
src/templating/tera.rs (1)
10-12: Align runtime module with stubs.You set the stub module but not the PyO3 class module. Add module to #[pyclass] to avoid mismatched module at runtime.
-#[pyclass] +#[pyclass(module = "oxapy.templating")] pub struct Tera {src/templating/minijinja.rs (2)
11-15: Align runtime module with stubs.Add module to #[pyclass] like in Jwt and Tera.
-#[pyclass] +#[pyclass(module = "oxapy.templating")] pub struct Jinja {
36-38: Leaked strings via Box::leak.This permanently leaks all template names/contents. If acceptable (long‑lived process), document it; else refactor to avoid 'static by storing owned Strings in the Environment (e.g., custom loader) or switching API usage.
.pre-commit-config.yaml (1)
14-16: Consider avoiding release builds in pre-commit.
maturin develop --releasewill slow commits. Use debug by default or make this hook manual.- name: Build (maturin develop --release) - entry: .venv/bin/maturin develop --release + name: Build (maturin develop) + entry: .venv/bin/maturin develop + stages: [manual]src/jwt.rs (1)
95-114: Avoid mutating caller’s dict and shadowing names.Mutating
claims["exp"]has side effects; alsoclaimsis re-used for different types.- let expiration = claims + let expiration = claims .get_item("exp")? .map(|exp| { exp.extract::<u64>() .map_err(|_| JwtError::new_err("Invalid `exp` format")) }) .transpose()? .unwrap_or(60); @@ - claims.set_item("exp", exp.as_secs())?; - - let Wrap::<Claims>(claims) = claims.try_into()?; + // Build a Rust Claims struct without mutating the input dict + let mut claims_copy = claims.copy()?; + claims_copy.set_item("exp", exp.as_secs())?; + let Wrap::<Claims>(claims_struct) = claims_copy.try_into()?; @@ - &claims, + &claims_struct,oxapy/jwt.pyi (1)
54-71: Minor stub doc nits.
- “Return Dictionary” → “dict”.
- Enumerate specific errors (JwtDecodingError, JwtInvalidAlgorithm) instead of generic “JwtError”.
src/multipart.rs (4)
62-65: Avoid extra allocation; return Bound directly.to_vec() copies the buffer. Use as_ref() and new_bound.
- fn content<'py>(&'py self, py: Python<'py>) -> Bound<'py, PyBytes> { - let data = &self.data.to_vec()[..]; - PyBytes::new(py, data) - } + fn content<'py>(&'py self, py: Python<'py>) -> Bound<'py, PyBytes> { + PyBytes::new_bound(py, self.data.as_ref()) + }
100-105: Handle quoted/positioned boundary parameter.Content-Type can be “multipart/form-data; boundary="abc"”. Current split may include quotes or fail if boundary isn’t last.
- let boundary = content_type - .split("boundary=") - .nth(1) - .map(|b| b.trim().to_string()) + let boundary = content_type + .split(';') + .find_map(|part| part.trim().strip_prefix("boundary=")) + .map(|b| b.trim_matches('"').to_string()) .ok_or_else(|| PyValueError::new_err("Boundary not found in Content-Type header"))?;
123-126: Missing field name silently stores under empty key.Using unwrap_or_default() can collapse multiple files into the same “” key. Consider erroring when name is absent.
- files.insert(field.name().unwrap_or_default().to_string(), file_obj); + let name = field.name().ok_or_else(|| PyValueError::new_err("File field missing name"))?; + files.insert(name.to_string(), file_obj);
85-88: Path safety and UX for save().Consider creating parent dirs and restricting to an allowed base to avoid path traversal, plus return a clearer error.
- fn save(&self, path: String) -> PyResult<()> { - std::fs::write(path, &self.data)?; + fn save(&self, path: String) -> PyResult<()> { + use std::path::{Path, PathBuf}; + let p = PathBuf::from(&path); + if let Some(parent) = p.parent() { std::fs::create_dir_all(parent)?; } + std::fs::write(&p, &self.data)?; Ok(()) }src/session.rs (2)
451-471: Cookie compliance: SameSite=None requires Secure.If cookie_same_site == "None" but cookie_secure == false, modern browsers may drop the cookie. Consider enforcing Secure when SameSite=None.
51-51: Renamecreate_at→created_at(or add a backward-compatible alias)src/session.rs defines #[pyo3(get)]
create_at: u64and the generated stub oxapy/init.pyi exposescreate_at; rename the Rust field tocreated_atand either add a deprecatedcreate_atgetter/property that returnscreated_at(preserves compatibility) or update all callers and regenerate stubs if you accept a breaking change.src/serializer/fields.rs (1)
110-169: Emit readOnly/writeOnly in JSON Schema and pre-size map accordinglyCurrently, read_only/write_only are exposed on Field but omitted from the generated schema. Also, capacity underestimates when
lengthis set (adds two keys). Suggest:
- Add readOnly/writeOnly when set.
- Fix capacity to account for
length(2 keys) and readOnly/writeOnly.Apply this diff:
- let capacity = 1 - + self.format.is_some() as usize - + self.min_length.is_some() as usize - + self.max_length.is_some() as usize - + self.pattern.is_some() as usize - + self.enum_values.is_some() as usize; + let capacity = 1 + + self.format.is_some() as usize + + (self.length.is_some() as usize) * 2 + + self.min_length.is_some() as usize + + self.max_length.is_some() as usize + + self.pattern.is_some() as usize + + self.enum_values.is_some() as usize + + self.read_only.is_some() as usize + + self.write_only.is_some() as usize; @@ if let Some(enum_values) = &self.enum_values { @@ schema.insert("enum".to_string(), Value::Array(enum_array)); } + + if let Some(ro) = self.read_only { + schema.insert("readOnly".to_string(), Value::Bool(ro)); + } + if let Some(wo) = self.write_only { + schema.insert("writeOnly".to_string(), Value::Bool(wo)); + }src/stub_gen.rs (1)
1-8: Allow overriding output root for stub generationProviding an env/CLI override reduces friction across environments (CI, local). Optional but handy.
Apply this diff:
use pyo3_stub_gen::Result; fn main() -> Result<()> { let stub = oxapy::stub_info()?; - // stub.python_root = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src"); + // Optionally override output root via env: + // if let Ok(root) = std::env::var("PY_STUB_ROOT") { + // stub.python_root = std::path::PathBuf::from(root); + // } stub.generate()?; Ok(()) }src/json.rs (1)
16-21: Avoid panic if orjson isn’t initialized; return a Python exception instead
ORJSON.get().unwrap()will panic ifinit_orjsonwasn’t called. Prefer a friendly PyErr.Apply this diff:
- let orjson_module = ORJSON.get().unwrap(); + let orjson_module = ORJSON + .get() + .ok_or_else(|| pyo3::exceptions::PyRuntimeError::new_err("orjson not initialized; call json::init_orjson(py) during module init"))?; @@ - let orjson_module = ORJSON.get().unwrap(); + let orjson_module = ORJSON + .get() + .ok_or_else(|| pyo3::exceptions::PyRuntimeError::new_err("orjson not initialized; call json::init_orjson(py) during module init"))?;Also applies to: 27-30
tests/test.py (2)
180-181: Relax serializer bench threshold to reduce CI flakiness200µs is fragile across shared CI runners. Suggest loosening.
Apply this diff:
- assert end - start < 0.0002 + assert end - start < 0.002
191-194: Relax response creation bench threshold20µs is very tight; transient noise can fail the test.
Apply this diff:
- assert end - start < 0.00002 + assert end - start < 0.0002src/cors.rs (3)
53-60: Default methods/headers should be lists of tokens (not a single comma-joined string)Current defaults produce the right header string after join, but make programmatic mutation awkward. Prefer tokenized defaults.
Apply this diff:
Self { origins: vec!["*".to_string()], - methods: vec!["GET, POST, PUT, DELETE, PATCH, OPTIONS".to_string()], - headers: vec!["Content-Type, Authorization, X-Requested-With, Accept".to_string()], + methods: vec![ + "GET".into(), + "POST".into(), + "PUT".into(), + "DELETE".into(), + "PATCH".into(), + "OPTIONS".into(), + ], + headers: vec![ + "Content-Type".into(), + "Authorization".into(), + "X-Requested-With".into(), + "Accept".into(), + ], allow_credentials: true, max_age: 86400, }
89-91: Drop unnecessary clone in reprFormatting doesn’t require cloning.
Apply this diff:
- fn __repr__(&self) -> String { - format!("{:#?}", self.clone()) - } + fn __repr__(&self) -> String { + format!("{:#?}", self) + }
122-126: apply_to_response doesn’t need PyResultThis path doesn’t touch Python state; consider returning
Responsedirectly. Optional.src/status.rs (1)
165-209: Consider adding int helper for Python ergonomicsSmall QoL: expose the numeric code via
__int__so Python users canint(Status.OK)and interoperate with libs expecting ints.#[pymethods] #[gen_stub_pymethods] impl Status { + fn __int__(&self) -> u16 { + *self as u16 + }oxapy/__init__.py (1)
1-29: Avoid star import; make all authoritative and handle optional jwtThis clears Ruff F403/F405, keeps
__all__in sync, and avoids exposing missingjwton aarch64.-from .oxapy import * - - -__all__ = ( - "HttpServer", - "Router", - "Status", - "Response", - "Request", - "Cors", - "Session", - "SessionStore", - "Redirect", - "File", - "get", - "post", - "delete", - "patch", - "put", - "head", - "options", - "static_file", - "catcher", - "convert_to_response", - "templating", - "serializer", - "exceptions", - "jwt", -) +from .oxapy import ( + HttpServer, + Router, + Status, + Response, + Request, + Cors, + Session, + SessionStore, + Redirect, + File, + get, + post, + delete, + patch, + put, + head, + options, + static_file, + catcher, + convert_to_response, + templating, + serializer, + exceptions, +) +# Optional on non-aarch64 +try: # pragma: no cover + from .oxapy import jwt as jwt # type: ignore[attr-defined] +except Exception: # pragma: no cover + jwt = None + +__all__ = [ + "HttpServer", + "Router", + "Status", + "Response", + "Request", + "Cors", + "Session", + "SessionStore", + "Redirect", + "File", + "get", + "post", + "delete", + "patch", + "put", + "head", + "options", + "static_file", + "catcher", + "convert_to_response", + "templating", + "serializer", + "exceptions", +] +if jwt is not None: + __all__.append("jwt")src/catcher.rs (1)
49-54: Validate handler is callable (avoid late failures)Right now any object can be stored. A lightweight check prevents surprises when invoking.
- fn __call__(&self, handler: Py<PyAny>) -> Catcher { - Catcher { - status: self.status, - handler, - } - } + fn __call__(&self, handler: Py<PyAny>) -> PyResult<Catcher> { + Python::with_gil(|py| { + if !handler.bind(py).is_callable() { + return Err(pyo3::exceptions::PyTypeError::new_err("handler must be callable")); + } + Ok(Catcher { + status: self.status, + handler, + }) + }) + }Note: This returns
PyResult<Catcher>; stubs should be updated accordingly.src/templating/mod.rs (1)
124-125: Fix error message“Not template” is unclear.
- .ok_or_else(|| PyValueError::new_err("Not template"))?; + .ok_or_else(|| PyValueError::new_err("No template engine configured on Request"))?;src/into_response.rs (5)
11-19: Avoid unnecessary clones in TryFromCheap win.
- Ok(Response { - status: Status::OK, - headers, - body: val.clone().into(), - }) + Ok(Response { status: Status::OK, headers, body: val.into() })
39-47: Avoid unnecessary clones in TryFrom<(String, Status)>Same as above.
- Ok(Response { - status: val.1, - headers, - body: val.0.clone().into(), - }) + Ok(Response { status: val.1, headers, body: val.0.into() })
64-74: From yields empty body but advertises JSONEither set a minimal valid JSON body or use
text/plain/no content type for empty bodies. Also avoidunwrap()in library code.- headers.insert(CONTENT_TYPE, "application/json".parse().unwrap()); - Response { + headers.insert(CONTENT_TYPE, "application/json".parse().expect("header parse")); + let mut resp = Response { status: val, headers, body: Bytes::new(), - } + }; + // Optionally: for non-204, include minimal JSON body + // if val != Status::NO_CONTENT { resp.set_body(r#"{"detail": ""}"#.to_string()); } + resp
100-106: 204 with JSON content-type is odd; drop content type for NO_CONTENTStarting from
NO_CONTENTwithapplication/jsonheader is misleading.- let mut response = Status::NO_CONTENT.into(); + let mut response = Status::NO_CONTENT.into(); + // Remove content-type for 204 responses + response.headers.remove(CONTENT_TYPE); val.apply_headers(&mut response); response
122-135: Improve “failed to convert” error diagnosticsInclude the actual Python type name to aid debugging.
- return Err(pyo3::exceptions::PyException::new_err( - "Failed to convert this type to response", - )); + let ty = result.bind($py).get_type().name().unwrap_or("unknown"); + return Err(pyo3::exceptions::PyException::new_err(format!( + "Failed to convert value of type '{}' to Response", + ty + )));src/response.rs (1)
77-85: Prefer precise body conversions; avoid generic PyAny::to_string().
to_string()may producerepr, not the original text. Handlestrexplicitly, fall back to__str__.- } else { - body.to_string().into() - }; + } else if let Ok(s) = body.extract::<String>() { + s.into() + } else { + body.str()?.to_string().into() + };src/request.rs (2)
216-226: Cookie parsing ignores whitespace; trim keys/values.Without trimming,
"Cookie: a=b; c=d"fails to match"c".- fn get_cookie(&self, name: &str) -> Option<&str> { + fn get_cookie(&self, name: &str) -> Option<&str> { let cookie = self.headers.get("cookie")?; let cookies = cookie.split(';'); for c in cookies { - let (k, v) = c.split_once('=')?; - if k == name { - return Some(v); + let (k, v) = c.split_once('=')?; + if k.trim() == name { + return Some(v.trim()); } } None }
237-246: Read-only guard refers to non-existent field "body".The struct has
data, notbody. Adjust the guard.- "method" | "uri" | "headers" | "body" | "template" => Err(PyException::new_err( + "method" | "uri" | "headers" | "data" | "template" => Err(PyException::new_err(src/lib.rs (2)
149-153: Convert IP parse errors to Python exceptions; avoid implicit ?.
ip.parse()?likely won’t auto-convert toPyErr. Use your helper.- addr: SocketAddr::new(ip.parse()?, port), + addr: SocketAddr::new(ip.parse().into_py_exception()?, port),
392-399: Ctrl+C handler starts a new Tokio runtime; prefer non-async send.
block_oninside a signal handler is heavy and can fail. Usetry_send()on a clone ofkill_tx, or a standardstd::sync::mpscsignal.oxapy/serializer.pyi (3)
266-281: Tighten data property typing to reflect actual return shape.
datayieldsdict | list[dict] | None. Consider:- def data(self) -> typing.Any: + def data(self) -> typing.Optional[typing.Union[dict, list[dict]]]:
384-403: Correct save() doc: it does not call is_valid().Runtime expects
validated_datato be set and raises if not.- Calls `is_valid()` first to populate `validated_data` before calling `create()`. + Requires `validated_data` to be set via `is_valid()`; this method does not call `is_valid()` automatically.
157-181: Align enum_values property type with constructor.Constructor accepts
Sequence[str]; property/settter should mirror that.- def enum_values(self) -> typing.Optional[builtins.list[builtins.str]]: ... + def enum_values(self) -> typing.Optional[typing.Sequence[builtins.str]]: ... @@ - def enum_values(self, value: typing.Optional[builtins.list[builtins.str]]) -> None: ... + def enum_values(self, value: typing.Optional[typing.Sequence[builtins.str]]) -> None: ...oxapy/__init__.pyi (1)
727-743: insert_header() doc promises chaining but returns None.Adjust doc to avoid misleading users.
- Returns: - Response: The response instance (for method chaining). + Returns: + None
📜 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 (33)
.pre-commit-config.yaml(1 hunks)Cargo.toml(1 hunks)README.md(1 hunks)oxapy/__init__.py(1 hunks)oxapy/__init__.pyi(1 hunks)oxapy/exceptions.pyi(1 hunks)oxapy/jwt.pyi(1 hunks)oxapy/serializer.pyi(1 hunks)oxapy/templating.pyi(1 hunks)pyproject.toml(1 hunks)src/catcher.rs(4 hunks)src/cors.rs(3 hunks)src/exceptions.rs(1 hunks)src/handling/mod.rs(0 hunks)src/handling/request_handler.rs(0 hunks)src/handling/response_handler.rs(0 hunks)src/into_response.rs(5 hunks)src/json.rs(1 hunks)src/jwt.rs(2 hunks)src/lib.rs(11 hunks)src/multipart.rs(4 hunks)src/request.rs(6 hunks)src/response.rs(6 hunks)src/routing.rs(8 hunks)src/serializer/fields.rs(6 hunks)src/serializer/mod.rs(17 hunks)src/session.rs(9 hunks)src/status.rs(3 hunks)src/stub_gen.rs(1 hunks)src/templating/minijinja.rs(1 hunks)src/templating/mod.rs(5 hunks)src/templating/tera.rs(1 hunks)tests/test.py(5 hunks)
💤 Files with no reviewable changes (3)
- src/handling/mod.rs
- src/handling/response_handler.rs
- src/handling/request_handler.rs
🧰 Additional context used
🧬 Code graph analysis (23)
src/stub_gen.rs (1)
src/lib.rs (1)
oxapy(546-577)
oxapy/jwt.pyi (1)
src/jwt.rs (2)
generate_token(95-124)verify_token(141-156)
src/json.rs (1)
src/serializer/mod.rs (1)
data(213-235)
oxapy/__init__.py (1)
src/lib.rs (1)
oxapy(546-577)
src/jwt.rs (1)
oxapy/jwt.pyi (1)
Jwt(6-71)
src/templating/minijinja.rs (2)
oxapy/__init__.pyi (1)
Jinja(431-433)oxapy/templating.pyi (1)
Jinja(62-68)
tests/test.py (4)
src/lib.rs (1)
oxapy(546-577)src/exceptions.rs (1)
exceptions(12-22)oxapy/serializer.pyi (2)
validated_data(260-260)validated_data(285-285)oxapy/__init__.pyi (1)
Response(643-765)
src/request.rs (2)
src/multipart.rs (1)
parse_multipart(96-132)oxapy/__init__.pyi (15)
json(567-588)File(105-176)session(613-637)Session(985-1122)SessionStore(1124-1261)name(135-135)Request(476-641)new(456-474)body(670-679)app_data(238-269)app_data(533-551)content_type(137-137)content_type(139-139)cookie_name(1159-1159)cookie_name(1173-1173)
src/exceptions.rs (1)
oxapy/exceptions.pyi (7)
UnauthorizedError(3-3)BaseError(1-1)ForbiddenError(4-4)NotFoundError(5-5)ConflictError(6-6)InternalError(7-7)BadRequestError(2-2)
src/templating/tera.rs (2)
oxapy/__init__.pyi (1)
Tera(1263-1265)oxapy/templating.pyi (1)
Tera(70-76)
src/into_response.rs (3)
src/response.rs (7)
body(77-77)body(78-78)body(104-106)try_from(256-260)headers(126-131)new(71-94)new(238-250)src/exceptions.rs (1)
exceptions(12-22)oxapy/exceptions.pyi (6)
BaseError(1-1)UnauthorizedError(3-3)ForbiddenError(4-4)NotFoundError(5-5)ConflictError(6-6)InternalError(7-7)
oxapy/templating.pyi (2)
src/lib.rs (1)
oxapy(546-577)oxapy/__init__.pyi (2)
Jinja(431-433)Tera(1263-1265)
src/templating/mod.rs (5)
oxapy/templating.pyi (3)
Template(8-76)Jinja(62-68)Tera(70-76)oxapy/__init__.pyi (6)
Jinja(431-433)Tera(1263-1265)new(456-474)render(433-433)render(1265-1265)render(1712-1740)src/lib.rs (11)
new(149-162)m(547-547)m(548-548)m(549-549)m(550-550)m(551-551)m(552-552)m(553-553)m(554-554)m(555-555)m(556-556)src/templating/minijinja.rs (2)
new(22-45)render(48-63)src/templating/tera.rs (2)
new(20-24)render(27-43)
oxapy/__init__.pyi (10)
src/lib.rs (5)
oxapy(546-577)value(518-518)template(263-265)run(368-381)new(149-162)src/templating/mod.rs (5)
templating(142-142)templating(143-143)templating(144-144)render(116-138)new(76-84)src/templating/minijinja.rs (3)
name(33-33)render(48-63)new(22-45)src/multipart.rs (2)
content(62-65)save(85-88)src/request.rs (6)
new(96-103)new(320-334)json(124-130)query(174-183)session(207-214)get_cookie(216-226)src/templating/tera.rs (2)
render(27-43)new(20-24)src/routing.rs (2)
new(57-63)static_file(491-529)src/session.rs (10)
new(73-86)new(362-381)get(102-116)remove(154-160)clear(175-182)values(204-208)get_session(393-412)clear_session(427-430)session_count(446-449)get_cookie_header(451-472)src/response.rs (8)
new(71-94)new(238-250)headers(126-131)body(77-77)body(78-78)body(104-106)insert_header(147-152)append_header(173-178)src/catcher.rs (1)
catcher(80-82)
src/session.rs (1)
oxapy/__init__.pyi (7)
get(906-916)get(1028-1044)get(1610-1625)keys(1096-1112)data(518-521)values(1113-1113)items(1114-1114)
src/catcher.rs (1)
oxapy/__init__.pyi (2)
CatcherBuilder(34-50)Status(1267-1566)
oxapy/serializer.pyi (3)
src/exceptions.rs (1)
exceptions(12-22)oxapy/exceptions.pyi (1)
BaseError(1-1)src/serializer/mod.rs (8)
data(213-235)schema(123-126)is_valid(141-154)validate(172-197)create(251-264)save(284-292)update(308-320)to_representation(333-373)
src/lib.rs (4)
src/request.rs (4)
new(96-103)new(320-334)app_data(149-151)session(207-214)src/session.rs (5)
new(73-86)new(362-381)py(226-226)get(102-116)get_cookie_header(451-472)src/middleware.rs (2)
new(16-20)new(28-30)src/into_response.rs (10)
from(65-73)from(77-97)from(101-105)convert_to_response(124-135)value(79-79)value(80-80)value(81-81)value(82-82)value(83-83)value(84-84)
src/serializer/mod.rs (3)
src/exceptions.rs (1)
exceptions(12-22)oxapy/exceptions.pyi (1)
BaseError(1-1)oxapy/serializer.pyi (17)
Field(134-212)many(147-147)many(171-171)context(264-264)context(289-289)read_only(159-159)read_only(183-183)write_only(161-161)write_only(185-185)Serializer(256-432)data(266-281)instance(258-258)instance(283-283)validated_data(260-260)validated_data(285-285)save(384-403)ValidationException(455-455)
src/routing.rs (1)
oxapy/__init__.pyi (2)
RouteBuilder(793-794)Router(796-983)
src/response.rs (2)
oxapy/__init__.pyi (4)
body(670-679)Response(643-765)Redirect(435-474)new(456-474)src/into_response.rs (4)
try_from(11-19)try_from(25-33)try_from(39-47)try_from(53-61)
src/cors.rs (1)
oxapy/__init__.pyi (1)
Response(643-765)
src/serializer/fields.rs (1)
oxapy/serializer.pyi (9)
required(139-139)required(163-163)nullable(143-143)nullable(167-167)format(145-145)format(169-169)many(147-147)many(171-171)Field(134-212)
🪛 markdownlint-cli2 (0.17.2)
README.md
12-12: Images should have alternate text (alt text)
(MD045, no-alt-text)
🪛 Ruff (0.12.2)
oxapy/__init__.py
1-1: from .oxapy import * used; unable to detect undefined names
(F403)
4-29: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
5-5: HttpServer may be undefined, or defined from star imports
(F405)
6-6: Router may be undefined, or defined from star imports
(F405)
7-7: Status may be undefined, or defined from star imports
(F405)
8-8: Response may be undefined, or defined from star imports
(F405)
9-9: Request may be undefined, or defined from star imports
(F405)
10-10: Cors may be undefined, or defined from star imports
(F405)
11-11: Session may be undefined, or defined from star imports
(F405)
12-12: SessionStore may be undefined, or defined from star imports
(F405)
13-13: Redirect may be undefined, or defined from star imports
(F405)
14-14: File may be undefined, or defined from star imports
(F405)
15-15: get may be undefined, or defined from star imports
(F405)
16-16: post may be undefined, or defined from star imports
(F405)
17-17: delete may be undefined, or defined from star imports
(F405)
18-18: patch may be undefined, or defined from star imports
(F405)
19-19: put may be undefined, or defined from star imports
(F405)
20-20: head may be undefined, or defined from star imports
(F405)
21-21: options may be undefined, or defined from star imports
(F405)
22-22: static_file may be undefined, or defined from star imports
(F405)
23-23: catcher may be undefined, or defined from star imports
(F405)
24-24: convert_to_response may be undefined, or defined from star imports
(F405)
25-25: templating may be undefined, or defined from star imports
(F405)
26-26: serializer may be undefined, or defined from star imports
(F405)
27-27: exceptions may be undefined, or defined from star imports
(F405)
28-28: jwt may be undefined, or defined from star imports
(F405)
⏰ 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). (14)
- GitHub Check: linux (ubuntu-22.04, armv7)
- GitHub Check: linux (ubuntu-22.04, ppc64le)
- GitHub Check: linux (ubuntu-22.04, x86)
- GitHub Check: windows (windows-latest, x86)
- GitHub Check: linux (ubuntu-22.04, aarch64)
- GitHub Check: linux (ubuntu-22.04, s390x)
- GitHub Check: musllinux (ubuntu-22.04, x86_64)
- GitHub Check: macos (macos-13, x86_64)
- GitHub Check: linux (ubuntu-22.04, x86_64)
- GitHub Check: macos (macos-14, aarch64)
- GitHub Check: musllinux (ubuntu-22.04, armv7)
- GitHub Check: windows (windows-latest, x64)
- GitHub Check: musllinux (ubuntu-22.04, aarch64)
- GitHub Check: musllinux (ubuntu-22.04, x86)
🔇 Additional comments (18)
oxapy/exceptions.pyi (1)
1-7: Stub hierarchy looks good.Matches typical HTTP error set and base error for reuse.
Confirm that the runtime module
oxapy.exceptionsregisters these classes so imports likefrom oxapy import exceptionswork without type-ignore.Cargo.toml (2)
29-29: Keepctrlcdependency — it's used (src/lib.rs:394:ctrlc::set_handler).Confirmed usage in src/lib.rs; leave the Cargo.toml entry.
40-41: Incorrect — jwt is already cfg-gatedsrc/lib.rs has #[cfg(not(target_arch = "aarch64"))] before both
mod jwt;(src/lib.rs:6–7) andjwt::jwt_submodule(m)?;(src/lib.rs:573–574), so the jsonwebtoken dependency in Cargo.toml is correctly gated and will not break aarch64 builds.Likely an incorrect or invalid review comment.
src/exceptions.rs (1)
6-9: New exceptions are wired correctly into the submodule.The added UnauthorizedError, ForbiddenError, NotFoundError, and ConflictError are properly created and exported via exceptions(). Looks good.
Also applies to: 16-19
src/multipart.rs (1)
96-96: Confirmed: no remainingparse_mutltipartreferences. Ripgrep returned 0 matches across the repo (35 files searched), so call‑sites have been updated toparse_multipart.src/serializer/fields.rs (4)
6-8: Module-scoped pyclass + stub macros: LGTMThe move to #[gen_stub_pyclass] and explicit module scoping looks correct and matches the stubbed API surface.
63-76: Constructor defaults now Optional[bool] with Some(...): LGTMDefaults align with oxapy/serializer.pyi (required=True, nullable=False, many=False).
155-166: Confirm intended semantics for nullable + many (both array and items allow null)Items inherit
nullable(type includes "null"), and the array itself is also nullable. Tests expect this, but please confirm this is the desired contract for all callers (Optional[List[Optional[T]]]) since it’s broader than Optional[List[T]].
216-229: No change required — PyO3 0.25 accepts (Self, Field) from #[new] for #[pyclass(extends=Field)]Confirmed: PyO3 0.25 allows #[new] to return (Self, Field) or PyResult<(Self, Field)>; that tuple implements Into<PyClassInitializer> so the current constructor pattern is valid.
src/json.rs (2)
14-14: Switch to Py: LGTMSignature aligns with the broader Py migration.
7-11: init_orjson is invoked during module init — verified
Found call at src/lib.rs:568:json::init_orjson(m.py())?src/status.rs (1)
29-33: Stub-gen attributes placement looks good
#[gen_stub_pyclass_enum]over the enum (with#[pyclass]) is consistent with the rest of the crate. No issues spotted.src/catcher.rs (2)
23-27: Public shape LGTM
Catcher { status, handler }withPy<PyAny>ownership is appropriate for crossing GIL boundaries.
79-82: Stub-gen annotation: good
#[gen_stub_pyfunction]aligns with the new builder flow; no issues.src/templating/mod.rs (1)
44-49: OK to use #[pyclass] on data-bearing enums (PyO3 0.25)
PyO3 0.25 supports complex (tuple/struct) enum variants as #[pyclass] — it generates Python variant classes, constructors and field getters, so no wrapper type is required.src/into_response.rs (1)
22-34: No GIL/serialization issue — json::dumps acquires the GIL internally.
src/json.rs definespub fn dumps(data: &Py<PyAny>) -> PyResult<String>and callsPython::with_gil(...), sojson::dumps(&val)is safe as written; no changes required.src/serializer/mod.rs (2)
213-235: data property: good many/one handling and PyAny interop.Returning
Py<PyAny>keeps flexibility and matches the stubs. The use ofto_representationper-instance looks correct.
468-495: Exception surface aligns with new oxapy.exceptions tree.
ValidationExceptionbase and export wiring look consistent withBaseError. No action needed.
| pyo3 = "=0.25.0" | ||
| pyo3-stub-gen = "0.13.1" | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Verify pyo3-stub-gen compatibility with pyo3 0.25.
Pinned pyo3 to 0.25.0; ensure pyo3-stub-gen 0.13.1 is compatible.
🌐 Web query:
Is pyo3-stub-gen 0.13.x compatible with PyO3 0.25?
💡 Result:
No — there is no published pyo3-stub-gen 0.13.x. The pyo3-stub-gen crate releases are <1.0 (latest listed ~0.9.1) while PyO3 0.25 is a published release; you should use the pyo3-stub-gen release compatible with the PyO3 tooling (check pyo3-build-config/pyo3 version requirements). Sources: pyo3-stub-gen crate/GitHub and PyO3 0.25 release. [1][2]
Sources
- pyo3-stub-gen (crate / GitHub). [1]
- PyO3 v0.25.0 release notes. [2]
Fix invalid pyo3-stub-gen version and align with PyO3 0.25.
- Cargo.toml (lines 18–20) currently has pyo3 = "=0.25.0" and pyo3-stub-gen = "0.13.1" — there is no published pyo3-stub-gen 0.13.x (latest published ~0.9.1).
- Action: replace the pyo3-stub-gen entry with a published version compatible with PyO3 0.25 (or adjust the PyO3 version to match), and verify compatibility via pyo3-stub-gen / PyO3 release notes or pyo3-build-config.
🤖 Prompt for AI Agents
In Cargo.toml around lines 18–20, the pyo3-stub-gen = "0.13.1" entry is invalid
for published crates and must be aligned with the PyO3 version; replace the
pyo3-stub-gen line with a published, compatible version (e.g., pyo3-stub-gen =
"0.9.1") or alternatively downgrade/upgrade the pyo3 entry so both versions
match according to pyo3-stub-gen release notes, then run cargo update/build and
verify compatibility (consult pyo3-stub-gen and PyO3 release notes or
pyo3-build-config if unsure).
| class Redirect(Response): | ||
| r""" | ||
| HTTP redirect response. | ||
|
|
||
| A specialized response type that redirects the client to a different URL. | ||
|
|
||
| Args: | ||
| location (str): The URL to redirect to. | ||
|
|
||
| Returns: | ||
| Redirect: A redirect response. | ||
|
|
||
| Example: | ||
| ```python | ||
| # Redirect to the home page | ||
| return Redirect("/home") | ||
|
|
||
| # Redirect to an external site | ||
| return Redirect("https://example.com") | ||
| ``` | ||
| """ | ||
| def new(self, location:builtins.str) -> Redirect: | ||
| r""" | ||
| Create a new HTTP redirect response. | ||
|
|
||
| Args: | ||
| location (str): The URL to redirect to. | ||
|
|
||
| Returns: | ||
| Redirect: A redirect response with status 301 (Moved Permanently). | ||
|
|
||
| Example: | ||
| ```python | ||
| # Redirect user after form submission | ||
| @router.post("/submit") | ||
| def submit_form(request): | ||
| # Process form... | ||
| return Redirect("/thank-you") | ||
| ``` | ||
| """ | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Redirect stub conflicts with runtime (not a Response subclass; new() returns tuple).
Rust returns (Redirect, Response) (see src/response.rs), not a Response-subclass instance. Fix class and method signature; mark as classmethod.
-class Redirect(Response):
+class Redirect:
@@
- def new(self, location:builtins.str) -> Redirect:
+ @classmethod
+ def new(cls, location:builtins.str) -> tuple[Redirect, Response]:
@@
- Returns:
- Redirect: A redirect response with status 301 (Moved Permanently).
+ Returns:
+ tuple[Redirect, Response]: Redirect marker and a 301 Response with Location header set.
@@
- ```python
- # Redirect user after form submission
- @router.post("/submit")
- def submit_form(request):
- # Process form...
- return Redirect("/thank-you")
- ```
+ ```python
+ # Redirect user after form submission
+ @router.post("/submit")
+ def submit_form(request):
+ _, resp = Redirect.new("/thank-you")
+ return resp
+ ```Run to confirm current signature:
🏁 Script executed:
#!/bin/bash
rg -nP -C3 '(?s)impl\s+Redirect\b.*?\bfn\s+new\s*\(' src/response.rsLength of output: 0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf "PWD: %s\n" "$(pwd)"
echo "----- rg occurrences of 'Redirect' (excluding target/node_modules) -----"
rg -n --hidden -S '\bRedirect\b' --glob '!**/target/**' --glob '!**/node_modules/**' || true
echo
echo "----- fd: find response.rs files -----"
fd -HI --type f -e rs response.rs || true
echo
echo "----- rg: Rust defs for Redirect (struct/enum/impl) -----"
rg -n --hidden -S '\b(pub\s+)?struct\s+Redirect\b|\benum\s+Redirect\b|\bimpl\s+Redirect\b' --glob '!**/target/**' || true
echo
echo "----- rg: Redirect::new or fn new occurrences -----"
rg -n --hidden -S 'Redirect::new|Redirect\s*::\s*new|fn\s+new\s*\(' --glob '!**/target/**' || true
echo
echo "----- Locate oxapy/__init__.pyi and print lines 430-480 if present -----"
pyfile=$(fd -HI --type f 'oxapy/__init__.pyi' || true)
if [ -n "$pyfile" ]; then
echo "Found: $pyfile"
sed -n '430,480p' "$pyfile" || true
else
echo "oxapy/__init__.pyi not found via fd; listing oxapy files:"
fd -HI --hidden --type f 'oxapy' || true
rg -n --hidden -S '^class Redirect' || true
fiLength of output: 913
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/response.rs"
printf "PWD: %s\n" "$(pwd)"
if [ -f "$file" ]; then
echo "Found $file"
echo
echo "----- occurrences with line numbers -----"
grep -nE '(\bstruct\s+Redirect\b|\benum\s+Redirect\b|\bimpl\s+Redirect\b|Redirect::new|\bfn\s+new\s*\()' "$file" || true
echo
echo "----- print surrounding context for 'Redirect' matches -----"
mapfile -t lines < <(grep -n 'Redirect' "$file" | cut -d: -f1 || true)
if [ ${#lines[@]} -eq 0 ]; then
echo "No raw 'Redirect' matches in $file"
else
for ln in "${lines[@]}"; do
start=$((ln-8)); if [ $start -lt 1 ]; then start=1; fi
end=$((ln+8))
echo "----- lines $start-$end -----"
sed -n "${start},${end}p" "$file"
echo
done
fi
else
echo "$file not found; listing src/ directory:"
ls -la src || true
fiLength of output: 331
Redirect stub conflicts with runtime — not a Response subclass; new() returns (Redirect, Response).
Update the pyi to match runtime: make Redirect not inherit Response and make new a @classmethod returning tuple[Redirect, Response] (see src/response.rs: fn new(...) -> (Redirect, Response)). File to change: oxapy/init.pyi (lines ~435-475).
-class Redirect(Response):
+class Redirect:
@@
- def new(self, location:builtins.str) -> Redirect:
+ @classmethod
+ def new(cls, location:builtins.str) -> tuple[Redirect, Response]:
@@
- Returns:
- Redirect: A redirect response with status 301 (Moved Permanently).
+ Returns:
+ tuple[Redirect, Response]: Redirect marker and a 301 Response with Location header set.
@@
- ```python
- # Redirect user after form submission
- @router.post("/submit")
- def submit_form(request):
- # Process form...
- return Redirect("/thank-you")
- ```
+ ```python
+ # Redirect user after form submission
+ @router.post("/submit")
+ def submit_form(request):
+ _, resp = Redirect.new("/thank-you")
+ return resp
+ ```🤖 Prompt for AI Agents
In oxapy/__init__.pyi around lines 435 to 475, the Redirect stub is incorrect:
it currently subclasses Response and defines new as an instance method returning
Redirect; update it to match runtime by removing the Response base class (make
Redirect a standalone class) and change new into a @classmethod that returns
tuple[Redirect, Response]; also adjust the docstring/example to show calling
Redirect.new(...) and returning the Response from the tuple.
| def render(request:Request, name:builtins.str, context:typing.Optional[dict]=None) -> Response: | ||
| r""" | ||
| Render a template and return the result as an HTTP response. | ||
|
|
||
| This function renders a template using the template engine configured for the request. | ||
|
|
||
| Args: | ||
| request (Request): The HTTP request object containing template configuration. | ||
| name (str): The name of the template to render. | ||
| context (dict, optional): Template variables to use during rendering. | ||
|
|
||
| Returns: | ||
| Response: An HTTP response with the rendered template as HTML. | ||
|
|
||
| Raises: | ||
| PyValueError: If no template engine is configured for the request. | ||
|
|
||
| Example: | ||
| ```python | ||
| from oxapy import Router | ||
| from oxapy import templating | ||
|
|
||
| router = Router() | ||
|
|
||
| @router.get("/") | ||
| def index(request): | ||
| return templating.render(request, "index.html", {"title": "Home Page"}) | ||
| ``` | ||
| """ | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Top-level render() likely not exported; stub may be incorrect.
src/lib.rs does not register a top-level render; it lives under oxapy.templating.render. Either re-export in Rust or remove this top-level stub.
🏁 Script executed:
#!/bin/bash
# Expect no results for a top-level render export
rg -n 'wrap_pyfunction!\(render' src/lib.rsLength of output: 106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "Listing repo root:"
ls -la | sed -n '1,200p'
echo
echo "Searching (fixed-string) for 'wrap_pyfunction!' occurrences:"
rg -nF 'wrap_pyfunction!' -S || true
echo
echo "Searching (fixed-string) for 'wrap_pyfunction!(' occurrences:"
rg -nF 'wrap_pyfunction!(' -S || true
echo
echo "Searching for add_function calls:"
rg -n --hidden --no-ignore -S 'add_function(' || true
echo
echo "Searching for #[pymodule] declarations:"
rg -n --hidden --no-ignore -S '\#\[\s*pymodule' || true
echo
echo "Searching for 'templating' symbol across repo:"
rg -n --hidden --no-ignore -S 'templating' || true
echo
echo "Searching for Rust 'pub fn render' definitions:"
rg -n --hidden --no-ignore -S 'pub fn render' || true
echo
echo "Searching for Python 'def render(' definitions:"
rg -n --hidden --no-ignore -S --type py '\bdef\s+render\s*\(' || true
echo
echo "Show oxapy/__init__.pyi around lines 1700-1750 if present:"
if [ -f oxapy/__init__.pyi ]; then
sed -n '1700,1750p' oxapy/__init__.pyi || true
else
echo "oxapy/__init__.pyi not found"
fi
echo
echo "Show src/lib.rs (first 400 lines) if present:"
if [ -f src/lib.rs ]; then
sed -n '1,400p' src/lib.rs || true
else
echo "src/lib.rs not found"
fiLength of output: 16223
Top-level render() stub is incorrect — remove or re-export
Rust registers a templating submodule (src/templating/mod.rs) and calls templating::templating_submodule(m)? in src/lib.rs (around line 569); there is no top-level render bound. Remove the top-level def render(...) in oxapy/init.pyi (lines 1712–1741) or add a top-level re-export/wrapper in src/lib.rs to expose render at oxapy.render.
🤖 Prompt for AI Agents
In oxapy/__init__.pyi around lines 1712 to 1741, the top-level def render(...)
stub is incorrect because the templating API is registered under the templating
submodule in Rust; either remove this top-level stub from the .pyi (delete lines
1712–1741) so consumers must import from oxapy.templating, or add a proper
top-level re-export/wrapper in Rust (edit src/lib.rs to call the templating
submodule's export function or expose a top-level render that delegates to
templating::render) and then update the .pyi to match that exported symbol;
ensure the chosen approach keeps signatures consistent and run
bindgen/type-check to update the stub accordingly.
| # This file is automatically generated by pyo3_stub_gen | ||
| # ruff: noqa: E501, F401 | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Ensure runtime export matches build cfg (aarch64).
jwt is conditionally added in Rust (not on aarch64). Verify oxapy/init.py doesn’t unconditionally import or re-export oxapy.jwt to avoid ImportError on aarch64.
🏁 Script executed:
#!/bin/bash
# Check for unconditional jwt imports/exports in runtime package
rg -n -C2 -P '(from\s+oxapy\s+import\s+jwt|import\s+oxapy\.jwt|__all__\s*=.*\bjwt\b)' oxapy/__init__.py || trueLength of output: 0
🏁 Script executed:
#!/bin/bash
# Re-run broader checks for unconditional jwt imports/exports and show relevant files if present.
# Avoid failing the whole script if no matches found.
set -uo pipefail
echo "=== locate oxapy/jwt.pyi ==="
find . -type f -path "*/oxapy/jwt.pyi" -print -exec printf "\n--- %s ---\n" {} \; -exec sed -n '1,200p' {} \; || true
echo -e "\n=== locate oxapy/__init__.py ==="
find . -type f -path "*/oxapy/__init__.py" -print -exec printf "\n--- %s ---\n" {} \; -exec sed -n '1,240p' {} \; || true
echo -e "\n=== search for direct imports/re-exports of jwt ==="
rg -n -C2 -P "(from\s+oxapy\s+import\s+jwt|import\s+oxapy\.jwt|__all__\s*=.*\bjwt\b)" || true
echo -e "\n=== search for any 'import oxapy.jwt' ==="
rg -n -C2 -P "import\s+oxapy\.jwt" || true
echo -e "\n=== search for any 'from oxapy import jwt' ==="
rg -n -C2 -P "from\s+oxapy\s+import\s+jwt" || true
echo -e "\n=== search for 'jwt' mentions inside oxapy package files ==="
rg -n -C2 -P "\bjwt\b" --hidden --glob "*/oxapy/**" || trueLength of output: 537
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== FILE: oxapy/jwt.pyi ==="
if [ -f "oxapy/jwt.pyi" ]; then
sed -n '1,200p' oxapy/jwt.pyi || true
else
echo "NOT FOUND: oxapy/jwt.pyi"
fi
echo -e "\n=== FILE: oxapy/__init__.py ==="
if [ -f "oxapy/__init__.py" ]; then
sed -n '1,400p' oxapy/__init__.py || true
else
echo "NOT FOUND: oxapy/__init__.py"
fi
echo -e "\n=== RG: direct import/re-export patterns in oxapy ==="
rg -n -C2 -P "(import\s+oxapy\.jwt|from\s+oxapy\s+import\s+jwt|\b__all__\s*=.*\bjwt\b)" oxapy || true
echo -e "\n=== RG: any 'jwt' token in oxapy package ==="
rg -n -C2 -P "\bjwt\b" oxapy || trueLength of output: 3720
Unconditionally exporting jwt — remove or guard it to avoid ImportError on aarch64
- oxapy/init.py (line 28): all includes "jwt" — remove "jwt" or conditionally add it (try/except import, platform check, or lazy import) because the Rust jwt extension is not built on aarch64.
- oxapy/init.pyi (line 7): contains
from . import jwt— update/remove the stub import so type stubs reflect runtime availability.
🤖 Prompt for AI Agents
oxapy/__init__.py (around line 28), oxapy/__init__.pyi (around line 7), and
oxapy/jwt.pyi (lines 1-3): the package unconditionally exports and stubs the
Rust-backed jwt module which isn't built on aarch64; remove "jwt" from __all__
or wrap the import/export in a try/except (or platform check) so import errors
are avoided at runtime, and update the .pyi stub to either remove the `from .
import jwt` line or make it conditional (reflecting runtime availability) so
type stubs match what can actually be imported on all platforms.
| def __new__(cls, data:typing.Optional[builtins.str]=None, instance:typing.Optional[typing.Any]=None, required:typing.Optional[builtins.bool]=True, nullable:typing.Optional[builtins.bool]=False, many:typing.Optional[builtins.bool]=False, context:typing.Optional[dict]=None, read_only:typing.Optional[builtins.bool]=False, write_only:typing.Optional[builtins.bool]=False) -> Object: | ||
| r""" | ||
| Create a new `Serializer` instance. | ||
|
|
||
| This constructor initializes the serializer with optional raw JSON data, an instance to serialize, | ||
| and optional context. The serializer is configured as a field of type `"object"`, with flags for | ||
| `required`, `nullable`, and `many`. | ||
|
|
||
| Args: | ||
| data (str, optional): Raw JSON string to be validated or deserialized. | ||
| instance (Any, optional): Python object instance to be serialized. | ||
| required (bool, optional): Whether the field is required (default: True). | ||
| nullable (bool, optional): Whether the field allows null values (default: False). | ||
| many (bool, optional): Whether the serializer handles multiple objects (default: False). | ||
| context (dict, optional): Additional context information. | ||
| read_only (bool, optional): If `True`, the serializer will be excluded when deserializing (default: False). | ||
| write_only (bool, optional): If `True`, the serializer will be excluded when serializing (default: False). | ||
|
|
||
| Returns: | ||
| Serializer: A tuple containing the serializer instance and its associated `Field`. | ||
|
|
||
| Example: | ||
| ```python | ||
| serializer, field = MySerializer( | ||
| data='{"email": "user@example.com", "password": "secret123"}' | ||
| ) | ||
| ``` | ||
| """ |
There was a problem hiding this comment.
Fix return type and doc of Serializer.new (undefined Object; wrong “tuple” claim).
-> Objectreferences an undefined type and breaks type checkers.- Runtime returns a
Serializerinstance, not a tuple.
Apply:
- def __new__(cls, data:typing.Optional[builtins.str]=None, instance:typing.Optional[typing.Any]=None, required:typing.Optional[builtins.bool]=True, nullable:typing.Optional[builtins.bool]=False, many:typing.Optional[builtins.bool]=False, context:typing.Optional[dict]=None, read_only:typing.Optional[builtins.bool]=False, write_only:typing.Optional[builtins.bool]=False) -> Object:
+ def __new__(cls, data:typing.Optional[builtins.str]=None, instance:typing.Optional[typing.Any]=None, required:typing.Optional[builtins.bool]=True, nullable:typing.Optional[builtins.bool]=False, many:typing.Optional[builtins.bool]=False, context:typing.Optional[dict]=None, read_only:typing.Optional[builtins.bool]=False, write_only:typing.Optional[builtins.bool]=False) -> Serializer:
@@
- Returns:
- Serializer: A tuple containing the serializer instance and its associated `Field`.
+ Returns:
+ Serializer: The serializer instance.
@@
- serializer, field = MySerializer(
- data='{"email": "user@example.com", "password": "secret123"}'
- )
+ serializer = MySerializer(
+ data='{"email": "user@example.com", "password": "secret123"}'
+ )📝 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.
| def __new__(cls, data:typing.Optional[builtins.str]=None, instance:typing.Optional[typing.Any]=None, required:typing.Optional[builtins.bool]=True, nullable:typing.Optional[builtins.bool]=False, many:typing.Optional[builtins.bool]=False, context:typing.Optional[dict]=None, read_only:typing.Optional[builtins.bool]=False, write_only:typing.Optional[builtins.bool]=False) -> Object: | |
| r""" | |
| Create a new `Serializer` instance. | |
| This constructor initializes the serializer with optional raw JSON data, an instance to serialize, | |
| and optional context. The serializer is configured as a field of type `"object"`, with flags for | |
| `required`, `nullable`, and `many`. | |
| Args: | |
| data (str, optional): Raw JSON string to be validated or deserialized. | |
| instance (Any, optional): Python object instance to be serialized. | |
| required (bool, optional): Whether the field is required (default: True). | |
| nullable (bool, optional): Whether the field allows null values (default: False). | |
| many (bool, optional): Whether the serializer handles multiple objects (default: False). | |
| context (dict, optional): Additional context information. | |
| read_only (bool, optional): If `True`, the serializer will be excluded when deserializing (default: False). | |
| write_only (bool, optional): If `True`, the serializer will be excluded when serializing (default: False). | |
| Returns: | |
| Serializer: A tuple containing the serializer instance and its associated `Field`. | |
| Example: | |
| ```python | |
| serializer, field = MySerializer( | |
| data='{"email": "user@example.com", "password": "secret123"}' | |
| ) | |
| ``` | |
| """ | |
| def __new__(cls, data:typing.Optional[builtins.str]=None, instance:typing.Optional[typing.Any]=None, required:typing.Optional[builtins.bool]=True, nullable:typing.Optional[builtins.bool]=False, many:typing.Optional[builtins.bool]=False, context:typing.Optional[dict]=None, read_only:typing.Optional[builtins.bool]=False, write_only:typing.Optional[builtins.bool]=False) -> Serializer: | |
| r""" | |
| Create a new `Serializer` instance. | |
| This constructor initializes the serializer with optional raw JSON data, an instance to serialize, | |
| and optional context. The serializer is configured as a field of type `"object"`, with flags for | |
| `required`, `nullable`, and `many`. | |
| Args: | |
| data (str, optional): Raw JSON string to be validated or deserialized. | |
| instance (Any, optional): Python object instance to be serialized. | |
| required (bool, optional): Whether the field is required (default: True). | |
| nullable (bool, optional): Whether the field allows null values (default: False). | |
| many (bool, optional): Whether the serializer handles multiple objects (default: False). | |
| context (dict, optional): Additional context information. | |
| read_only (bool, optional): If `True`, the serializer will be excluded when deserializing (default: False). | |
| write_only (bool, optional): If `True`, the serializer will be excluded when serializing (default: False). | |
| Returns: | |
| Serializer: The serializer instance. | |
| Example: |
🤖 Prompt for AI Agents
In oxapy/serializer.pyi around lines 290 to 317, the __new__ signature and
docstring are incorrect: the return annotation "-> Object" references an
undefined type and the doc claims it returns a tuple. Change the return type
annotation to the Serializer class (e.g., "-> Serializer") and update the
Returns section of the docstring to state it returns a Serializer instance (not
a tuple); remove or correct any mention of `Object` and ensure examples/doc text
reflect a single Serializer return value.
| impl Request { | ||
| pub(crate) async fn handle( | ||
| self, | ||
| RequestContext { | ||
| request_sender, | ||
| routers, | ||
| channel_capacity, | ||
| cors, | ||
| catchers, | ||
| }: RequestContext, | ||
| ) -> Result<hyper::Response<Full<Bytes>>, hyper::http::Error> { | ||
| for router in routers { | ||
| if let Some(match_route) = router.find(&self.method, &self.uri) { | ||
| let (tx, mut rx) = tokio::sync::mpsc::channel(channel_capacity); | ||
| let transmutate_route: MatchRoute = unsafe { std::mem::transmute(match_route) }; | ||
|
|
||
| let process_request = ProcessRequest { | ||
| tx, | ||
| cors: cors.clone(), | ||
| catchers: catchers.clone(), | ||
| router: Some(router), | ||
| match_route: Some(transmutate_route), | ||
| request: Arc::new(self.clone()), | ||
| }; | ||
|
|
||
| if request_sender.send(process_request).await.is_ok() { | ||
| if let Some(response) = rx.recv().await { | ||
| return response.try_into(); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| let (tx, mut rx) = tokio::sync::mpsc::channel(channel_capacity); | ||
|
|
||
| let process_request = ProcessRequest { | ||
| tx, | ||
| cors, | ||
| catchers, | ||
| router: None, | ||
| match_route: None, | ||
| request: Arc::new(self), | ||
| }; | ||
|
|
||
| if request_sender.send(process_request).await.is_ok() { | ||
| if let Some(response) = rx.recv().await { | ||
| return response.try_into(); | ||
| } | ||
| } | ||
|
|
||
| let response: Response = Status::NOT_FOUND.into(); | ||
| response.try_into() | ||
| } |
There was a problem hiding this comment.
Unsound lifetime transmute; pass owned params/handler instead.
This repeats the unsound 'static transmute. Use the owned route match returned by a safe find_owned().
- if let Some(match_route) = router.find(&self.method, &self.uri) {
+ if let Some((params, handler, router_arc)) = router.find_owned(&self.method, &self.uri)
+ {
let (tx, mut rx) = tokio::sync::mpsc::channel(channel_capacity);
- let transmutate_route: MatchRoute = unsafe { std::mem::transmute(match_route) };
let process_request = ProcessRequest {
tx,
cors: cors.clone(),
catchers: catchers.clone(),
- router: Some(router),
- match_route: Some(transmutate_route),
+ router: Some(router_arc),
+ params: Some(params),
+ handler: Some(handler),
request: Arc::new(self.clone()),
};Committable suggestion skipped: line range outside the PR's diff.
| pub async fn build(self) -> PyResult<Request> { | ||
| let mut request = Request::new(self.method, self.uri, self.headers); | ||
|
|
||
| let bytes = self.req.collect().await.into_py_exception()?.to_bytes(); | ||
| let body = String::from_utf8_lossy(&bytes).to_string(); | ||
|
|
||
| if !body.is_empty() { | ||
| request.data = Some(body.clone()); | ||
| } | ||
|
|
||
| if let Some(content_type) = request.headers.get("content-type") { | ||
| if content_type.starts_with("multipart/form-data") { | ||
| let parsed_multipart = parse_multipart(content_type, bytes) | ||
| .await | ||
| .into_py_exception()?; | ||
| request.form = Some(parsed_multipart.fields); | ||
| request.files = Some(parsed_multipart.files); | ||
| } | ||
| } | ||
|
|
||
| if let Some(store) = self.session_store { | ||
| if let Some(session_id) = | ||
| get_session_id(request.headers.get("cookie"), &store.cookie_name) | ||
| { | ||
| if let Ok(session) = store.get_session(Some(session_id)) { | ||
| request.session = Some(Arc::new(session)); | ||
| request.session_store = Some(store.clone()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| request.app_data = self.app_data; | ||
| request.template = self.template; | ||
|
|
||
| Ok(request) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Graceful body/build errors; avoid unwrap.
build().await.unwrap() will panic on malformed requests or parser errors. Return 500 instead.
Apply in lib.rs service closure (see dedicated comment), or change RequestBuilder::build callers to handle Err and convert to Response.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo root: $(pwd)"
echo "--- Searching for direct unwrap/expect on build().await ---"
rg -n --hidden -S '\.build\(\)\.await\.(unwrap|expect)\b' || true
echo "--- Searching for any .build().await occurrences (context 3 lines) ---"
rg -n --hidden -S '\.build\(\)\.await' -C3 || true
echo "--- Searching for RequestBuilder::build references ---"
rg -n --hidden -S 'RequestBuilder::build' -C3 || true
echo "--- Searching for lib.rs and likely service closure entrypoints ---"
rg -n --hidden -S '(lib\.rs|make_service_fn|service_fn|hyper::service|tower::Service)' -C3 || trueLength of output: 1042
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo root: $(pwd)"
echo "---- lib.rs (400-480) ----"
nl -ba src/lib.rs | sed -n '400,480p' || true
echo "---- search for build().await (flexible whitespace) ----"
rg -n --hidden -S 'build\(\)\s*\.?\s*await' -C3 || true
echo "---- search for RequestBuilder references ----"
rg -n --hidden -S 'RequestBuilder' -C3 || trueLength of output: 1633
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo root: $(pwd)"
echo "---- lib.rs 420-520 ----"
sed -n '420,520p' src/lib.rs || true
echo
echo "---- Search for .build().await patterns ----"
rg -n --hidden -S '\.build\(\)\s*\.await' -C3 || true
rg -n --hidden -S '\.build\(\)\s*\.await\.(unwrap|expect)\b' -C3 || true
echo
echo "---- Search for any .await.unwrap/expect usages ----"
rg -n --hidden -S '\.await\.(unwrap|expect)\b' -C3 || true
echo
echo "---- Search for RequestBuilder usages ----"
rg -n --hidden -S 'RequestBuilder::new' -C5 || true
rg -n --hidden -S 'RequestBuilder' -C3 || trueLength of output: 7091
Return 500 on RequestBuilder::build errors — remove .unwrap()
Replace the .build().await.unwrap() in the service closure with explicit error handling that converts Err into an HTTP 500 Response (or routes to the existing catcher). Location: src/lib.rs:443–448.
- Also audit other unwraps in the same closure (e.g.
acquire_owned().await.unwrap(),listener.accept().await.unwrap()) and handle or justify them.
🤖 Prompt for AI Agents
In src/request.rs lines 351 to 386: the RequestBuilder::build call can fail and
callers currently use .build().await.unwrap() in src/lib.rs around lines
443–448; change the service closure to explicitly handle build() returning Err
by mapping it to an HTTP 500 response (or routing to the existing catcher)
instead of unwrapping. Replace the .await.unwrap() with a match or ?-style
handling that converts the PyErr into a Response with status 500 and an
appropriate body/header, and propagate/convert other fallible calls in that same
closure (e.g. acquire_owned().await.unwrap(), listener.accept().await.unwrap())
into proper error handling or explicit error responses/early returns so no
unwrap() panics remain; audit each unwrap in the closure and either handle the
error, return a 500 Response, or add a justified, documented unwrap if it truly
cannot fail.
| fn set(&self, key: &str, value: Py<PyAny>) -> PyResult<()> { | ||
| let mut data = self.data.write().into_py_exception()?; | ||
| data.insert(key.to_string(), value); | ||
| *self.modified.lock().unwrap() = true; |
There was a problem hiding this comment.
Don’t unwrap poisoned locks (propagate as Python error).
Use into_py_exception() consistently.
- *self.modified.lock().unwrap() = true;
+ *self.modified.lock().into_py_exception()? = true;📝 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.
| *self.modified.lock().unwrap() = true; | |
| *self.modified.lock().into_py_exception()? = true; |
🤖 Prompt for AI Agents
In src/session.rs around line 136, avoid unwrapping the mutex (which can panic
on poison); change "*self.modified.lock().unwrap() = true;" to acquire the lock
with error propagation to Python instead of unwrap, e.g. use
self.modified.lock().map_err(|e| e.into_py_exception("Mutex poisoned"))? and
then set the value, ensuring you call into_py_exception() to convert the poison
error into a PyErr and return it to Python rather than panicking.
| fn __iter__(slf: PyRef<'_, Self>, py: Python<'_>) -> PyResult<Py<PyAny>> { | ||
| let keys = slf.keys(py)?; | ||
| let iter_func = py.get_type::<PyTuple>().call_method1("__iter__", (keys,))?; | ||
| iter_func.into_py_any(py) | ||
| } |
There was a problem hiding this comment.
iter calls tuple.iter on a list — TypeError at runtime.
keys() returns a list; calling tuple.iter(list) is invalid. Call the list’s iter instead.
- fn __iter__(slf: PyRef<'_, Self>, py: Python<'_>) -> PyResult<Py<PyAny>> {
- let keys = slf.keys(py)?;
- let iter_func = py.get_type::<PyTuple>().call_method1("__iter__", (keys,))?;
- iter_func.into_py_any(py)
- }
+ fn __iter__(slf: PyRef<'_, Self>, py: Python<'_>) -> PyResult<Py<PyAny>> {
+ let keys = slf.keys(py)?;
+ let iter = keys.as_ref(py).call_method0("__iter__")?;
+ iter.into_py_any(py)
+ }📝 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.
| fn __iter__(slf: PyRef<'_, Self>, py: Python<'_>) -> PyResult<Py<PyAny>> { | |
| let keys = slf.keys(py)?; | |
| let iter_func = py.get_type::<PyTuple>().call_method1("__iter__", (keys,))?; | |
| iter_func.into_py_any(py) | |
| } | |
| fn __iter__(slf: PyRef<'_, Self>, py: Python<'_>) -> PyResult<Py<PyAny>> { | |
| let keys = slf.keys(py)?; | |
| let iter = keys.as_ref(py).call_method0("__iter__")?; | |
| iter.into_py_any(py) | |
| } |
🤖 Prompt for AI Agents
In src/session.rs around lines 224 to 228, keys() returns a Python list but the
code calls tuple.__iter__(keys), causing a TypeError; replace that call with the
list's iterator instead (e.g., call keys.call_method0("__iter__") or use
py.get_type::<PyList>().call_method1("__iter__", (keys,))) and return that
iterator as Py<PyAny>, preserving error propagation.
| templating.add_class::<tera::Tera>()?; | ||
| templating.add_class::<minijinja::Jinja>()?; | ||
| m.add_function(wrap_pyfunction!(render, m)?)?; | ||
| m.add_submodule(&templating) |
There was a problem hiding this comment.
render is registered on the root module but docs and .pyi suggest oxapy.templating.render
Either export under the submodule or update docs/types. Recommend adding to the submodule (and optionally keep a root alias for BC).
- m.add_function(wrap_pyfunction!(render, m)?)?;
+ // Expose under submodule to match docs and pyi
+ templating.add_function(wrap_pyfunction!(render, templating)?)?;
+ // Optional: also expose at root for convenience/BC
+ // m.add_function(wrap_pyfunction!(render, m)?)?;Also add "render" to oxapy/__init__.py’s __all__ if you keep the root alias.
📝 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.
| templating.add_class::<tera::Tera>()?; | |
| templating.add_class::<minijinja::Jinja>()?; | |
| m.add_function(wrap_pyfunction!(render, m)?)?; | |
| m.add_submodule(&templating) | |
| templating.add_class::<tera::Tera>()?; | |
| templating.add_class::<minijinja::Jinja>()?; | |
| // Expose under submodule to match docs and pyi | |
| templating.add_function(wrap_pyfunction!(render, templating)?)?; | |
| // Optional: also expose at root for convenience/BC | |
| // m.add_function(wrap_pyfunction!(render, m)?)?; | |
| m.add_submodule(&templating) |
🤖 Prompt for AI Agents
In src/templating/mod.rs around lines 143 to 146, the render function is being
registered on the root module but the docs and .pyi indicate it should live
under oxapy.templating; move the wrap_pyfunction!(render, m) registration into
the templating submodule (i.e., call
templating.add_function(wrap_pyfunction!(render, &templating)?)) so render is
exported from the templating submodule, and optionally keep a root-level alias
by adding m.add_function(wrap_pyfunction!(render, m)?)? if you want backwards
compatibility; if you keep the root alias also add "render" to
oxapy/__init__.py's __all__ to reflect the exported symbol.
1 participant
Summary by CodeRabbit
New Features
Documentation
Tests
Chores