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

allow user to pass middleware options #442

Merged
merged 9 commits into from
Apr 26, 2024
Merged

Conversation

geospatial-jeff
Copy link
Collaborator

@geospatial-jeff geospatial-jeff commented Aug 2, 2022

Related Issue(s):

Description:
The goal of this PR is to make it easier for users to configure middleware passed to the application. Starlette / fastapi expect the middleware class and arguments to that class to be passed separately, the starlette.middleware.Middleware class encapsulates this.

  • Reworks middleware injection to allow user to pass arguments to each middleware. This is a breaking change to the type signature of StacApi.middleware instance variable (List[type] -> List[Middleware])
  • Adds the StacApi.add_middleware for adding middleware to the underlying fastapi application. This method uses the same business logic as Starlette.add_middleware.

Before

from stac_fastapi.api.app import StacApi
from starlette.middleware.cors import CORSMiddleware

# No way to specify middleware arguments.
api = StacApi(
    middlewares=[CORSMiddleware]
)

After

from stac_fastapi.api.app import StacApi
from starlette.middleware import Middleware
from starlette.middleware.cors import CORSMiddleware

# Create middleware w/ options.
cors_middleware = Middleware(
    CORSMiddleware,
    allow_origins=("*",),
    allow_methods=("POST", "GET",)
)

# Add in StacApi constructor.
api = StacApi(
    middleware=[cors_middleware]
)

# Or add after-the-fact.
api.add_middleware(cors_middleware)

PR Checklist:

  • Code is formatted and linted (run pre-commit run --all-files)
  • Tests pass (run make test)
  • Documentation has been updated to reflect changes, if applicable, and docs build successfully (run make docs)
  • Changes are added to the CHANGELOG.

@geospatial-jeff geospatial-jeff changed the title allow user to pass middleware options to stac api constructor, proxy … allow user to pass middleware options Aug 2, 2022
@geospatial-jeff geospatial-jeff mentioned this pull request Aug 2, 2022
4 tasks
@gadomski
Copy link
Member

@geospatial-jeff is this PR still a good change? If so, since it's breaking, we should decide if we want it to go in before or after #450.

@gadomski
Copy link
Member

After looking at this again, IMO this is a non-breaking change to the type signature of middlewares, since the contained type of the list was undefined previously, and the default argument made it pretty clear what that type should be. So IMO this would be ok to include in 2.5.

@gadomski gadomski added this to the 2.5.0 milestone Jan 31, 2023
@jonhealy1 jonhealy1 self-requested a review March 29, 2024 04:42
Copy link
Collaborator

@jonhealy1 jonhealy1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice work here!

@vincentsarago
Copy link
Member

Let's hold of til #625 get's merged but this PR is ✅

@vincentsarago vincentsarago self-requested a review April 4, 2024 22:02
@vincentsarago vincentsarago merged commit e7f82d6 into main Apr 26, 2024
7 checks passed
@vincentsarago vincentsarago deleted the middleware-injection branch April 26, 2024 08:12
@pedro-cf
Copy link

Greetings @vincentsarago @geospatial-jeff @jonhealy1,

How do I convert this type of middleware definition with these new changes?

@app.middleware("http")
async def add_process_time_header(request: Request, call_next):
    start_time = time.time()
    response = await call_next(request)
    process_time = time.time() - start_time
    response.headers["X-Process-Time"] = str(process_time)
    return response

@vincentsarago
Copy link
Member

@pedro-cf using http middleware is deprecated (or at least not encouraged by starlette). you can easily create a pure ASGI middleware like https://github.com/developmentseed/titiler/blob/main/src/titiler/core/titiler/core/middleware.py#L66-L100 which will be easier to use in stac-fastapi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better middleware injection
5 participants