Skip to content

Commit

Permalink
Replace starlette-context with BaseHTTPMiddleware
Browse files Browse the repository at this point in the history
this is still not ideal, as the entire response will be loaded into
memory. This is a problem with streaming responses.
encode/starlette#1012 (comment)
is actually enlightening here:

> "This means this class will either load the entirety of streaming requests into memory (this issue) and run the background before returning the response (galaxyproject#919 ), or if we fix those problems, that it will then encourage users to leave resources in a pending or open state, an arguably worse result. In short, it's problematic."

Which is exactly what we'd be doing with a middleware, keeping resources
open for longer than necessary, which we need to avoid if we ever want
to run background / async tasks.  I think the solution here what we
already had, path operation dependencies.
  • Loading branch information
mvdbeek committed Feb 20, 2021
1 parent 871c27e commit 2545965
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 24 deletions.
1 change: 0 additions & 1 deletion lib/galaxy/dependencies/dev-requirements.txt
Expand Up @@ -209,7 +209,6 @@ sqlalchemy==1.3.22; (python_version >= "2.7" and python_full_version < "3.0.0")
sqlitedict==1.7.0
sqlparse==0.4.1; python_version >= "3.5"
starlette==0.13.6; python_version >= "3.6" and python_version < "4.0"
starlette-context==0.3.1; python_version >= "3.7"
stevedore==3.3.0; python_version >= "3.6"
svgwrite==1.4.1; python_version >= "3.6"
tempita==0.5.2
Expand Down
1 change: 0 additions & 1 deletion lib/galaxy/dependencies/pinned-requirements.txt
Expand Up @@ -169,7 +169,6 @@ sqlalchemy==1.3.22; (python_version >= "2.7" and python_full_version < "3.0.0")
sqlitedict==1.7.0
sqlparse==0.4.1; python_version >= "3.5"
starlette==0.13.6; python_version >= "3.6" and python_version < "4.0"
starlette-context==0.3.1; python_version >= "3.7"
stevedore==3.3.0; python_version >= "3.6"
svgwrite==1.4.1; python_version >= "3.6"
tempita==0.5.2
Expand Down
1 change: 1 addition & 0 deletions lib/galaxy/model/base.py
Expand Up @@ -14,6 +14,7 @@
scoped_session,
sessionmaker
)

from galaxy.util.bunch import Bunch

REQUEST_ID = ContextVar('request_id', default=None)
Expand Down
37 changes: 18 additions & 19 deletions lib/galaxy/webapps/galaxy/fast_app.py
@@ -1,12 +1,12 @@
from uuid import uuid4

from fastapi import FastAPI, Request
from fastapi.exceptions import RequestValidationError
from fastapi.middleware.wsgi import WSGIMiddleware
from fastapi.responses import JSONResponse
from starlette.middleware.base import BaseHTTPMiddleware
from starlette.middleware.cors import CORSMiddleware
from starlette.responses import Response
from starlette_context import plugins
from starlette_context import context
from starlette_context.middleware import RawContextMiddleware

from galaxy.exceptions import MessageException
from galaxy.web.framework.base import walk_controller_modules
Expand Down Expand Up @@ -115,31 +115,30 @@ async def preflight_handler(request: Request, rest_of_path: str) -> Response:
return response


class SqlSessionPlugin(plugins.Plugin):
"""
Stores X-Request-ID set by the RequestIdPlugin in a separate contexvar
and sets and unsets the session.
"""
key = 'X-Request-ID'
def add_sa_session_middleware(app: FastAPI, gx_app):
class CustomHeaderMiddleware(BaseHTTPMiddleware):

def __init__(self, *args, gx_app=None, **kwargs):
super().__init__(*args, **kwargs)
self.gx_app = gx_app
def __init__(self, app, gx_app) -> None:
super().__init__(app)
self.gx_app = gx_app

async def process_request(self, request):
self.gx_app.model.set_request_id(context.data[self.key])
return context.data['X-Request-ID']
async def dispatch(self, request, call_next):
request_id = uuid4().hex
self.gx_app.model.set_request_id(request_id)
try:
response = await call_next(request)
finally:
self.gx_app.model.unset_request_id(request_id)
return response

async def enrich_response(self, arg) -> None:
self.gx_app.model.unset_request_id(context.data[self.key])
app.add_middleware(CustomHeaderMiddleware, gx_app=gx_app)


def initialize_fast_app(gx_webapp, gx_app):
app = FastAPI(
openapi_tags=api_tags_metadata
)
app.add_middleware(RawContextMiddleware, plugins=(SqlSessionPlugin(gx_app=gx_app),))
app.add_middleware(RawContextMiddleware, plugins=(plugins.RequestIdPlugin(force_new_uuid=True),))
add_sa_session_middleware(app, gx_app)
add_exception_handler(app)
add_galaxy_middleware(app, gx_app)
wsgi_handler = WSGIMiddleware(gx_webapp)
Expand Down
1 change: 0 additions & 1 deletion pyproject.toml
Expand Up @@ -70,7 +70,6 @@ sqlalchemy-migrate = "*"
SQLAlchemy-Utils = "!=0.36.8" # https://github.com/kvesteri/sqlalchemy-utils/issues/462
sqlitedict = "*"
sqlparse = "*"
starlette-context = "*"
svgwrite = "*"
uvicorn = "*"
WebOb = "*"
Expand Down
4 changes: 2 additions & 2 deletions test/unit/api/test_fastapi.py
Expand Up @@ -10,7 +10,6 @@
add_exception_handler,
add_sa_session_middleware,
)

from ..unittest_utils.galaxy_mock import MockApp

app = FastAPI()
Expand All @@ -28,6 +27,7 @@ def is_valid_uuid(val):
class UnexpectedException(Exception):
pass


def get_app():
global GX_APP
if GX_APP is None:
Expand Down Expand Up @@ -64,4 +64,4 @@ def test_sa_session_middleware():
assert gx_app.model.scoped_registry.registry == {}
with pytest.raises(UnexpectedException):
response = client.get("/internal_server_error")
assert gx_app.model.scoped_registry.registry == {}
assert gx_app.model.scoped_registry.registry == {}

0 comments on commit 2545965

Please sign in to comment.