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

fix(api)!: use root path for prefixing #241

Merged
merged 12 commits into from
Nov 1, 2023
Merged

Conversation

anayeaye
Copy link
Collaborator

@anayeaye anayeaye commented Oct 24, 2023

What

  • Use fast-api root path configuration and mangum api_base_path instead of overwriting the api host in the http lambda integration

    • Use FastAPI root_path for optional base path configuration and, if provided, also set proxy url in additional servers property
    • Set api_gateway_base_path=root_path in handler
  • Simplify custom domain management

    • If using cloudfront, do not create custom api-specific subdomains--just use the default apigateway urls route based on root path settings provided in raster and stac api configuration.

How tested

Temporary stacks are running to test the two supported path configurations:

  1. Using an in-account custom subdomain (test-raster) for the raster API: https://test-raster.delta-backend.com/docs
  2. Using a cloudfront and custom root path (/api/raster) for the raster API: https://alex.delta-backend.xyz/api/raster/docs

@anayeaye anayeaye marked this pull request as draft October 24, 2023 21:11
@anayeaye anayeaye changed the title WIP fix(apis)! use root path for prefixing fix(apis)! use root path for prefixing Oct 24, 2023
@anayeaye anayeaye changed the title fix(apis)! use root path for prefixing fix! use root path for prefixing Oct 24, 2023
@anayeaye anayeaye changed the title fix! use root path for prefixing fix(api)!: use root path for prefixing Oct 24, 2023
@anayeaye anayeaye marked this pull request as ready for review October 30, 2023 19:19
logging.getLogger("mangum.lifespan").setLevel(logging.ERROR)
logging.getLogger("mangum.http").setLevel(logging.ERROR)

handler = Mangum(app, lifespan="auto")
handler = Mangum(app, lifespan="auto", api_gateway_base_path=settings.root_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@smohiudd @ividito: after making 👆 suggested change, I was also able to remove the additional fast api additional servers configuration (sharing because was a red flag in reviews yesterday)

@anayeaye anayeaye merged commit dd00f5a into develop Nov 1, 2023
4 checks passed
@anayeaye anayeaye deleted the fix/raster-stac-search branch November 1, 2023 20:14
anayeaye added a commit that referenced this pull request Nov 13, 2023
…cloudfront (#244)

# What
- adds optional configuration to deploy all endpoints with a custom root
path (i.e. `/api/stac` and `/api/raster`) #229, #241
- adds support for using a cloudfront as a reverse proxy #229, #241,
#245
- titiler-pgstac upgraded from 0.2.3 to 0.8.0 #239
- titiler upgrade for custom colormap configuration #243

# How tested
A temporary test stack was deployed to confirm that raster-api works as
expected and that the two-subdomain staging-stac and staging-raster
pattern is still supported. The [pre-deploy diff action for this
pr](https://github.com/NASA-IMPACT/veda-backend/actions/runs/6791564314/job/18463354484#step:11:919)
confirms that the raster API lambda will be upgraded and no domain name
changes will be caused.
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.

4 participants