Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python ASGI: support bytearray, memoryview in response body #648

Open
filiphanes opened this issue Feb 16, 2022 · 1 comment
Open

Python ASGI: support bytearray, memoryview in response body #648

filiphanes opened this issue Feb 16, 2022 · 1 comment
Labels
z-enhancement ⬆️ Product Enhancement

Comments

@filiphanes
Copy link

filiphanes commented Feb 16, 2022

When sending ASGI response:

await send({
    "type": "http.response.body",
    "body": body,
    "more_body": False,
})

Nginx Unit currently allows body to be onlybytes type, but Python types bytearray and memoryview could be supported, to avoid conversion to bytes and copying memory.
bytearray is faster then bytes when appending lots of small parts.
memoryview over bytes or bytearray objects is used to avoid memory copying.

Maybe all objects with buffer protocol could be supported https://docs.python.org/3/c-api/buffer.html

Looking at file https://github.com/nginx/unit/blob/master/src/python/nxt_python_asgi_http.c
and
https://docs.python.org/3/c-api/memoryview.html
https://docs.python.org/3/c-api/bytearray.html
this should be straightforward.

Uvicorn ASGI server support these types.

@VBart VBart added the z-enhancement ⬆️ Product Enhancement label Feb 16, 2022
@andrey-zelenkov
Copy link
Contributor

Hi @filiphanes,

Thank you for your report. Could you please try following patch with bytearray support if it works for you:

diff --git a/src/python/nxt_python_asgi_http.c b/src/python/nxt_python_asgi_http.c
--- a/src/python/nxt_python_asgi_http.c
+++ b/src/python/nxt_python_asgi_http.c
@@ -362,16 +362,6 @@ nxt_py_asgi_http_response_body(nxt_py_as
     Py_ssize_t              body_len, body_off;
     nxt_py_asgi_ctx_data_t  *ctx_data;
 
-    body = PyDict_GetItem(dict, nxt_py_body_str);
-    if (nxt_slow_path(body != NULL && !PyBytes_Check(body))) {
-        return PyErr_Format(PyExc_TypeError, "'body' is not a byte string");
-    }
-
-    more_body = PyDict_GetItem(dict, nxt_py_more_body_str);
-    if (nxt_slow_path(more_body != NULL && !PyBool_Check(more_body))) {
-        return PyErr_Format(PyExc_TypeError, "'more_body' is not a bool");
-    }
-
     if (nxt_slow_path(http->complete)) {
         return PyErr_Format(PyExc_RuntimeError,
                             "Unexpected ASGI message 'http.response.body' "
@@ -382,9 +372,26 @@ nxt_py_asgi_http_response_body(nxt_py_as
         return PyErr_Format(PyExc_RuntimeError, "Concurrent send");
     }
 
+    more_body = PyDict_GetItem(dict, nxt_py_more_body_str);
+    if (nxt_slow_path(more_body != NULL && !PyBool_Check(more_body))) {
+        return PyErr_Format(PyExc_TypeError, "'more_body' is not a bool");
+    }
+
+    body = PyDict_GetItem(dict, nxt_py_body_str);
+
     if (body != NULL) {
-        body_str = PyBytes_AS_STRING(body);
-        body_len = PyBytes_GET_SIZE(body);
+        if (PyBytes_Check(body)) {
+            body_str = PyBytes_AS_STRING(body);
+            body_len = PyBytes_GET_SIZE(body);
+
+        } else if (PyByteArray_Check(body)) {
+            body_str = PyByteArray_AS_STRING(body);
+            body_len = PyByteArray_GET_SIZE(body);
+
+        } else {
+            return PyErr_Format(PyExc_TypeError,
+                                "'body' is not a byte string or bytearray");
+        }
 
         nxt_unit_req_debug(http->req, "asgi_http_response_body: %d, %d",
                            (int) body_len, (more_body == Py_True) );

andrey-zelenkov added a commit to andrey-zelenkov/unit that referenced this issue Jan 26, 2024
andrey-zelenkov added a commit to andrey-zelenkov/unit that referenced this issue Jan 30, 2024
@filiphanes requested support for bytearray
and memoryview in the request body here:
nginx#648

This patch implements bytearray body support only.
Memoryview body still need to be implemented.
andrey-zelenkov added a commit to andrey-zelenkov/unit that referenced this issue Feb 9, 2024
@filiphanes requested support for bytearray
and memoryview in the request body here:
<nginx#648>

This patch implements bytearray body support only.
Memoryview body still need to be implemented.
andrey-zelenkov added a commit that referenced this issue Feb 21, 2024
@filiphanes requested support for bytearray
and memoryview in the request body here:
<#648>

This patch implements bytearray body support only.
Memoryview body still need to be implemented.
andrey-zelenkov added a commit that referenced this issue Feb 27, 2024
@filiphanes requested support for bytearray
and memoryview in the request body here:
<#648>

This patch implements bytearray body support only.
Memoryview body still need to be implemented.
@callahad callahad moved this to 📖 Backlog in NGINX Unit Planning May 20, 2024
pkillarjun pushed a commit to pkillarjun/unit that referenced this issue May 29, 2024
@filiphanes requested support for bytearray
and memoryview in the request body here:
<nginx#648>

This patch implements bytearray body support only.
Memoryview body still need to be implemented.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
z-enhancement ⬆️ Product Enhancement
Projects
Status: 📖 Backlog
Development

No branches or pull requests

3 participants