Unverified Commit 4f76e35f authored by Burak's avatar Burak Committed by GitHub
Browse files

Python: map Python middlewares to Tower layers (#1871)



* Python: map Python middlewares to Tower layers

* Make middleware layer infallible

* Use message and status code from `PyMiddlewareException`

* Introduce `FuncMetadata` to represent some information about a Python function

* Improve middleware errors

* Add missing copyright headers

* Allow accessing and changing request body

* Allow changing response

* Add some documentation about moving data back-and-forth between Rust and Python

* Add `mypy` to Pokemon service and update typings and comments for middlewares

* Add or update comments on the important types

* Add Rust equivalent of `collections.abc.MutableMapping`

* Add `PyHeaderMap` to make `HeaderMap` accessible from Python

* Apply suggestions from code review

Co-authored-by: default avatarLuca Palmieri <20745048+LukeMathWalker@users.noreply.github.com>

* Improve logging

* Add `RichPyErr` to have a better output for `PyErr`s

* Better error messages for `PyMiddlewareError` variants

* Factor out repeating patterns in tests

* Preserve `__builtins__` in `globals` to fix tests in Python 3.7.10 (our CI version)

* Export `RichPyErr` to fix `cargo doc` error

* Apply suggestions from code review

Co-authored-by: default avatarMatteo Bigoi <1781140+crisidev@users.noreply.github.com>

* Add missing SPDX headers

* Document that `keys`, `values` and `items` on `PyMutableMapping` causes clones

Co-authored-by: default avatarLuca Palmieri <20745048+LukeMathWalker@users.noreply.github.com>
Co-authored-by: default avatarMatteo Bigoi <1781140+crisidev@users.noreply.github.com>
parent b82b6a65
Loading
Loading
Loading
Loading
+28 −16
Original line number Diff line number Diff line
@@ -107,7 +107,7 @@ class PythonApplicationGenerator(
            ##[derive(Debug)]
            pub struct App {
                handlers: #{HashMap}<String, #{SmithyPython}::PyHandler>,
                middlewares: #{SmithyPython}::PyMiddlewares,
                middlewares: Vec<#{SmithyPython}::PyMiddlewareHandler>,
                context: Option<#{pyo3}::PyObject>,
                workers: #{parking_lot}::Mutex<Vec<#{pyo3}::PyObject>>,
            }
@@ -141,7 +141,7 @@ class PythonApplicationGenerator(
                fn default() -> Self {
                    Self {
                        handlers: Default::default(),
                        middlewares: #{SmithyPython}::PyMiddlewares::new::<#{Protocol}>(vec![]),
                        middlewares: vec![],
                        context: None,
                        workers: #{parking_lot}::Mutex::new(vec![]),
                    }
@@ -171,9 +171,6 @@ class PythonApplicationGenerator(
                fn handlers(&mut self) -> &mut #{HashMap}<String, #{SmithyPython}::PyHandler> {
                    &mut self.handlers
                }
                fn middlewares(&mut self) -> &mut #{SmithyPython}::PyMiddlewares {
                    &mut self.middlewares
                }
                """,
                *codegenScope,
            )
@@ -212,13 +209,22 @@ class PythonApplicationGenerator(
                }
                rustTemplate(
                    """
                    let middleware_locals = #{pyo3_asyncio}::TaskLocals::new(event_loop);
                    let service = #{tower}::ServiceBuilder::new()
                        .boxed_clone()
                        .layer(
                            #{SmithyPython}::PyMiddlewareLayer::<#{Protocol}>::new(self.middlewares.clone(), middleware_locals),
                        )
                        .service(builder.build());
                    let mut service = #{tower}::util::BoxCloneService::new(builder.build());

                    {
                        use #{tower}::Layer;
                        #{tracing}::trace!("adding middlewares to rust python router");
                        let mut middlewares = self.middlewares.clone();
                        // Reverse the middlewares, so they run with same order as they defined
                        middlewares.reverse();
                        for handler in middlewares {
                            #{tracing}::trace!(name = &handler.name, "adding python middleware");
                            let locals = #{pyo3_asyncio}::TaskLocals::new(event_loop);
                            let layer = #{SmithyPython}::PyMiddlewareLayer::<#{Protocol}>::new(handler, locals);
                            service = #{tower}::util::BoxCloneService::new(layer.layer(service));
                        }
                    }

                    Ok(service)
                    """,
                    "Protocol" to protocol.markerStruct(),
@@ -248,11 +254,17 @@ class PythonApplicationGenerator(
                pub fn context(&mut self, context: #{pyo3}::PyObject) {
                   self.context = Some(context);
                }
                /// Register a request middleware function that will be run inside a Tower layer, without cloning the body.
                /// Register a Python function to be executed inside a Tower middleware layer.
                ##[pyo3(text_signature = "(${'$'}self, func)")]
                pub fn request_middleware(&mut self, py: #{pyo3}::Python, func: #{pyo3}::PyObject) -> #{pyo3}::PyResult<()> {
                    use #{SmithyPython}::PyApp;
                    self.register_middleware(py, func, #{SmithyPython}::PyMiddlewareType::Request)
                pub fn middleware(&mut self, py: #{pyo3}::Python, func: #{pyo3}::PyObject) -> #{pyo3}::PyResult<()> {
                    let handler = #{SmithyPython}::PyMiddlewareHandler::new(py, func)?;
                    #{tracing}::trace!(
                        name = &handler.name,
                        is_coroutine = handler.is_coroutine,
                        "registering middleware function",
                    );
                    self.middlewares.push(handler);
                    Ok(())
                }
                /// Main entrypoint: start the server on multiple workers.
                ##[pyo3(text_signature = "(${'$'}self, address, port, backlog, workers)")]
+0 −1
Original line number Diff line number Diff line
@@ -152,7 +152,6 @@ class PythonServerModuleGenerator(
            middleware.add_class::<#{SmithyPython}::PyRequest>()?;
            middleware.add_class::<#{SmithyPython}::PyResponse>()?;
            middleware.add_class::<#{SmithyPython}::PyMiddlewareException>()?;
            middleware.add_class::<#{SmithyPython}::PyHttpVersion>()?;
            pyo3::py_run!(
                py,
                middleware,
+5 −11
Original line number Diff line number Diff line
@@ -91,7 +91,7 @@ class PythonServerOperationHandlerGenerator(
        writable {
            rustTemplate(
                """
                #{tracing}::debug!("Executing Python handler function `$name()`");
                #{tracing}::trace!(name = "$name", "executing python handler function");
                #{pyo3}::Python::with_gil(|py| {
                    let pyhandler: &#{pyo3}::types::PyFunction = handler.extract(py)?;
                    let output = if handler.args == 1 {
@@ -110,7 +110,7 @@ class PythonServerOperationHandlerGenerator(
        writable {
            rustTemplate(
                """
                #{tracing}::debug!("Executing Python handler coroutine `$name()`");
                #{tracing}::trace!(name = "$name", "executing python handler coroutine");
                let result = #{pyo3}::Python::with_gil(|py| {
                    let pyhandler: &#{pyo3}::types::PyFunction = handler.extract(py)?;
                    let coroutine = if handler.args == 1 {
@@ -132,15 +132,9 @@ class PythonServerOperationHandlerGenerator(
                """
                // Catch and record a Python traceback.
                result.map_err(|e| {
                    let traceback = #{pyo3}::Python::with_gil(|py| {
                        match e.traceback(py) {
                            Some(t) => t.format().unwrap_or_else(|e| e.to_string()),
                            None => "Unknown traceback\n".to_string()
                        }
                    });
                    let error = e.into();
                    #{tracing}::error!("{}{}", traceback, error);
                    error
                    let rich_py_err = #{SmithyPython}::rich_py_err(#{pyo3}::Python::with_gil(|py| { e.clone_ref(py) }));
                    #{tracing}::error!(error = ?rich_py_err, "handler error");
                    e.into()
                })
                """,
                *codegenScope,
+8 −0
Original line number Diff line number Diff line
@@ -41,6 +41,14 @@ tracing-appender = { version = "0.2.2"}
[dev-dependencies]
pretty_assertions = "1"
futures-util = "0.3"
tower-test = "0.4"
tokio-test = "0.4"
pyo3-asyncio = { version = "0.17.0", features = ["testing", "attributes", "tokio-runtime"] }

[[test]]
name = "middleware_tests"
path = "src/middleware/pytests/harness.rs"
harness = false

[package.metadata.docs.rs]
all-features = true
+3 −0
Original line number Diff line number Diff line
@@ -31,6 +31,9 @@ build: codegen
	cargo build
	ln -sf $(DEBUG_SHARED_LIBRARY_SRC) $(SHARED_LIBRARY_DST)

py_check: build
	mypy pokemon_service.py

release: codegen
	cargo build --release
	ln -sf $(RELEASE_SHARED_LIBRARY_SRC) $(SHARED_LIBRARY_DST)
Loading