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

base_url used to build the urls without root_path #767

Open
ccancellieri opened this issue Dec 9, 2024 · 6 comments
Open

base_url used to build the urls without root_path #767

ccancellieri opened this issue Dec 9, 2024 · 6 comments

Comments

@ccancellieri
Copy link

Hi all,
I need to have multiple instances of this api under the same fastAPI and I'm trying to understand why base_url has been used to generate urls not using also:
f"{base_url}/{request.scope.get("root_path")}

This is preventing me to mount the application under a specific fastAPI path: all the generated urls are not shipping the root_path infact.

It works with a fixed prefix but it is kind of hardcoded routing base url which prevents me to properly use fastAPI with this library.

It would be great if we could centralise the base url management here to also use the root_path from the scope:
def get_base_url(request: Request) -> str:

My goal is to mount multiple times this library to be able to have a first level of catalogs in a single application:

"links": [
{
"rel": "self",
"type": "application/json",
"href": "http://localhost:8080/catalogs/catalog_1/"
},
{
"rel": "root",
"type": "application/json",
"href": "http://localhost:8080/catalogs/catalog_1/"
},
{
"rel": "data",
"type": "application/json",
"href": "http://localhost:8080/catalogs/catalog_1/collections"
},
{
"rel": "conformance",
"type": "application/json",
"title": "STAC/WFS3 conformance classes implemented by this server",
"href": "http://localhost:8080/catalogs/catalog_1/conformance"
},
{
"rel": "search",
"type": "application/geo+json",
"title": "STAC search",
"href": "http://localhost:8080/catalogs/catalog_1/search",
"method": "GET"
},

Related problem:
stac-utils/stac-fastapi-elasticsearch-opensearch#308 (comment)

Related questions: why we are using the state of the application to save it?
app.state.router_prefix=....

What do you think?

@vincentsarago
Copy link
Member

Hi @ccancellieri,

I'm not sure to fully get everything. We've had numerous report related to prefix and route_path and usually it always comes down to proxy settings https://github.com/stac-utils/stac-fastapi/issues?q=is%3Aissue+root_path+is%3Aclosed

Could you be a bit more precise how you are mounting multiple stac-fastapi application?

root_path isn't a settings option in the current stac-fastapi application, I've created #769 to resolve this, but I'm not sure this will fix your issue

Related questions: why we are using the state of the application to save it?

This is a legacy code and I will be worry to change this logic for now

@ccancellieri
Copy link
Author

Hi @ccancellieri,

I'm not sure to fully get everything. We've had numerous report related to prefix and route_path and usually it always comes down to proxy settings https://github.com/stac-utils/stac-fastapi/issues?q=is%3Aissue+root_path+is%3Aclosed

Could you be a bit more precise how you are mounting multiple stac-fastapi application?

This is what I'm trying to obtain:
https://github.com/ccancellieri/stac-fastapi-elasticsearch-opensearch/blob/c838d13d5f349e209cc6198781fa488173fc84d8/stac_fastapi/elasticsearch/stac_fastapi/elasticsearch/app.py#L150

... and here is why:
stac-utils/stac-fastapi-elasticsearch-opensearch#308 (comment)

root_path isn't a settings option in the current stac-fastapi application, I've created #769 to resolve this, but I'm not sure this will fix your issue

Thank you for this!

@vincentsarago
Copy link
Member

@ccancellieri
Copy link
Author

ccancellieri commented Jan 20, 2025

Unfortunately Collections uses the base_url to calculate the links:

href=urljoin(self.base_url, f"collections/{self.collection_id}"),

and router_prefix is not affecting this.
I could be wrong but debugging this is what I see.
I'm testing several options with no success

I think the unique option to correctly mount as a catalog this application is to use root_path or request.url to build the links to make it working behind a mounted context.

Current code:
urljoin(self.base_url,f"collections/{self.collection_id}")

Result (wrong):

'http://localhost:8080/collections/1e8874ad-d2b1-4eba-a881-7c8f092adffd'

Working but clunky:
urljoin(f"{self.base_url}/{self.request.scope.get('root_path')}", f"collections/{self.collection_id}")

Result (correct):
'http://localhost:8080/catalogs/catalog_1/collections/1e8874ad-d2b1-4eba-a881-7c8f092adffd'

Working and flexible:
urljoin(self.url, f"collections/{self.collection_id}")

Result (correct):
'http://localhost:8080/catalogs/catalog_1/collections/1e8874ad-d2b1-4eba-a881-7c8f092adffd'

thanks again.

I could try to produce a fix and a POC if it coud help.

@vincentsarago
Copy link
Member

🤔 in https://github.com/stac-utils/stac-fastapi/pull/769/files#diff-1eec7e8fe8a9e8a3a1060ba4ff8263360ccaca01a419ab18179e33379a097d35 we added a test to make sure the root_path was properly set to the base_url so I'm not quite sure what to do next

@ccancellieri
Copy link
Author

Dear @vincentsarago thank you so much for your great effort.
I think you correctly now passed the root_path.
Unfortunately this does't solve my specific problem since root_path is correctly set but unused here:

def get_base_url(request: Request) -> str:

using root_path instead of the state of the app here is key to solve the wrong generation of the urls in all the extensions.

I think I could produce a pull request to show the use case to help you to understand the specific use case (multiple intance of a stac-fastapi mount).

This issue is resolving a quite important problem to me: having /catalogs/{catalog_name}/... prepended to each link for each instance of a stac_fastapi instance.

You could proceed merging this improvement as it could be used as base for the changes to the get_base_url, I will then be able to discuss this change in a specific issue.

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

No branches or pull requests

2 participants