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

Add ROOT_PATH env variable to simplify running Titiler behind a proxy #343

Merged
merged 6 commits into from
Jul 27, 2021

Conversation

tayden
Copy link
Contributor

@tayden tayden commented Jul 22, 2021

I'd really like to get Titiler running on an existing server, and have requests to the root path /titiler passed to a running docker container running on, e.g. port 9000. However, there's currently no way to specify the root_path parameter in the FastAPI instantiation, so the app has been required to run at the root location /. (see FastAPI docs on running behind a proxy)

This PR allows users to set the root_path parameter of the FastAPI instance via an environment variable so that it's dead simple to run the service behind a reverse proxy. It defaults that root_path location to "/", so nothing anybody currently has running breaks.

I suspect that for this to be accepted, some documentation has to be written up for this behaviour. I'm just not sure where that should go. If someone wants to tell me where to put that, I can fill in those details.

@vincentsarago
Copy link
Member

vincentsarago commented Jul 22, 2021

@tayden thanks for the PR

Would it make more sense to add this to https://github.com/developmentseed/titiler/blob/master/src/titiler/application/titiler/application/settings.py#L6 ?

also looking at the CI, the tests fails because this adds a / to the base path, so I guess the default root_path should be ""

@tayden
Copy link
Contributor Author

tayden commented Jul 22, 2021

@vincentsarago That makes sense to me. I've updated/fixed the code

@vincentsarago
Copy link
Member

excellent, thanks @tayden

Tests are failing but it's unrelated to the changes (and have been fixed on the master branch just now). Could you merge from master and add a note in the changelog 🙏

@tayden tayden requested a review from vincentsarago July 26, 2021 19:23
@vincentsarago
Copy link
Member

🙏 thanks @tayden

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.

2 participants