From c087c385316e83c5c7a9f224608886c8418485ef Mon Sep 17 00:00:00 2001 From: Erik Aker Date: Mon, 10 Aug 2020 18:50:48 -0700 Subject: [PATCH 1/3] Version 0.13.8 release Revert Queue maxsize fix in BaseHTTPMiddleware --- docs/release-notes.md | 7 +++++++ starlette/__init__.py | 2 +- starlette/middleware/base.py | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/docs/release-notes.md b/docs/release-notes.md index 6ba60435a..478bb89dc 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -1,3 +1,10 @@ +## 0.13.8 + +* Revert `Queue(maxsize=1)` fix for `BaseHTTPMiddleware` middleware classes and streaming responses. + * Before this fix in version `0.13.7`, users were relying on behavior where the response object is fully evaluated within their middleware based on `BaseHTTPMiddleware`. The `maxsize=1` fix released in version `0.13.7` _broke_ that expectation and made it more likely that users would have confusing pending tasks accumulate. As a result, using `BaseHTTPMiddleware` middleware classes with streaming responses is currently _not recommended_. See issues [#1022](https://github.com/encode/starlette/issues/1022), [#1012](https://github.com/encode/starlette/issues/1012), and [#919](https://github.com/encode/starlette/issues/919). Moving forward, this middleware class will likely need to be redesigned and for now it is recommended that instead of subclassing `BaseHTTPMiddleware`, users consider writing Middleware classes similar to [other examples](https://github.com/encode/starlette/blob/master/starlette/middleware/authentication.py#L14) in the codebase. + +* The `StaticFiles` constructor now allows `pathlib.Path` in addition to strings for its `directory` argument. + ## 0.13.7 * Fix high memory usage when using `BaseHTTPMiddleware` middleware classes and streaming responses. diff --git a/starlette/__init__.py b/starlette/__init__.py index af935ca6d..496382738 100644 --- a/starlette/__init__.py +++ b/starlette/__init__.py @@ -1 +1 @@ -__version__ = "0.13.7" +__version__ = "0.13.8" diff --git a/starlette/middleware/base.py b/starlette/middleware/base.py index d0089c0c9..b347a6a2d 100644 --- a/starlette/middleware/base.py +++ b/starlette/middleware/base.py @@ -27,7 +27,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: async def call_next(self, request: Request) -> Response: loop = asyncio.get_event_loop() - queue: "asyncio.Queue[typing.Optional[Message]]" = asyncio.Queue(maxsize=1) + queue: "asyncio.Queue[typing.Optional[Message]]" = asyncio.Queue() scope = request.scope receive = request.receive From b4f74cb7c363fc27872e687c661233d94208be54 Mon Sep 17 00:00:00 2001 From: Erik Aker Date: Mon, 10 Aug 2020 20:55:37 -0700 Subject: [PATCH 2/3] Reword reasoning for reverted change --- docs/release-notes.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/release-notes.md b/docs/release-notes.md index 478bb89dc..0964d1dae 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -1,7 +1,9 @@ ## 0.13.8 * Revert `Queue(maxsize=1)` fix for `BaseHTTPMiddleware` middleware classes and streaming responses. - * Before this fix in version `0.13.7`, users were relying on behavior where the response object is fully evaluated within their middleware based on `BaseHTTPMiddleware`. The `maxsize=1` fix released in version `0.13.7` _broke_ that expectation and made it more likely that users would have confusing pending tasks accumulate. As a result, using `BaseHTTPMiddleware` middleware classes with streaming responses is currently _not recommended_. See issues [#1022](https://github.com/encode/starlette/issues/1022), [#1012](https://github.com/encode/starlette/issues/1012), and [#919](https://github.com/encode/starlette/issues/919). Moving forward, this middleware class will likely need to be redesigned and for now it is recommended that instead of subclassing `BaseHTTPMiddleware`, users consider writing Middleware classes similar to [other examples](https://github.com/encode/starlette/blob/master/starlette/middleware/authentication.py#L14) in the codebase. + * Before version `0.13.7`, [some users](https://github.com/encode/starlette/issues/1022) were relying on behavior where the response object would be fully evaluated within their middleware based on `BaseHTTPMiddleware`. (Of course, this was a problem for `StreamingResponse`s with this middleware, because they were also being loaded fully into memory.) The `maxsize=1` fix released in version `0.13.7` explicitly prevented the response object from being fully evaluated until later `await` calls, but any users who were dropping this response for any other created inside their middleware were likely to see pending tasks accumulate. + + * Moving forward, using `BaseHTTPMiddleware` middleware classes with streaming responses is **not recommended** See issues [#1022](https://github.com/encode/starlette/issues/1022), [#1012](https://github.com/encode/starlette/issues/1012), and [#919](https://github.com/encode/starlette/issues/919). Further, this middleware class will likely need to be redesigned and for now it is recommended that instead of subclassing `BaseHTTPMiddleware`, users consider writing middleware classes similar to [other examples](https://github.com/encode/starlette/blob/master/starlette/middleware/authentication.py#L14) in the codebase. * The `StaticFiles` constructor now allows `pathlib.Path` in addition to strings for its `directory` argument. From 0ee90363008abfbe870c16905ec26880b4e606de Mon Sep 17 00:00:00 2001 From: Erik Aker Date: Thu, 13 Aug 2020 07:44:29 -0700 Subject: [PATCH 3/3] Pull extended note on maxsize fix --- docs/release-notes.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/docs/release-notes.md b/docs/release-notes.md index 0964d1dae..caadb7a62 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -1,9 +1,6 @@ ## 0.13.8 -* Revert `Queue(maxsize=1)` fix for `BaseHTTPMiddleware` middleware classes and streaming responses. - * Before version `0.13.7`, [some users](https://github.com/encode/starlette/issues/1022) were relying on behavior where the response object would be fully evaluated within their middleware based on `BaseHTTPMiddleware`. (Of course, this was a problem for `StreamingResponse`s with this middleware, because they were also being loaded fully into memory.) The `maxsize=1` fix released in version `0.13.7` explicitly prevented the response object from being fully evaluated until later `await` calls, but any users who were dropping this response for any other created inside their middleware were likely to see pending tasks accumulate. - - * Moving forward, using `BaseHTTPMiddleware` middleware classes with streaming responses is **not recommended** See issues [#1022](https://github.com/encode/starlette/issues/1022), [#1012](https://github.com/encode/starlette/issues/1012), and [#919](https://github.com/encode/starlette/issues/919). Further, this middleware class will likely need to be redesigned and for now it is recommended that instead of subclassing `BaseHTTPMiddleware`, users consider writing middleware classes similar to [other examples](https://github.com/encode/starlette/blob/master/starlette/middleware/authentication.py#L14) in the codebase. +* Revert `Queue(maxsize=1)` fix for `BaseHTTPMiddleware` middleware classes and streaming responses. * The `StaticFiles` constructor now allows `pathlib.Path` in addition to strings for its `directory` argument.