-
Notifications
You must be signed in to change notification settings - Fork 176
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
feat: STAC Render Extension support #1038
feat: STAC Render Extension support #1038
Conversation
Render extension started during STAC render sprint in SatSummit Lisbon 2024. - listing (or showing to please Vincent) Please contribute to complete the feature to - generate the final XYZ link for rendering following the rules in STAC extensions - add a dedicated endpoint for render XYZ
Latest changes:
Both are based on item-steninel2.json
Request:
For the |
dep.query_params, render | ||
) | ||
_ = dependency(**query_values) | ||
query.update(query_values) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you think about moving this outside the extensions package to titiler.core.utils?
so we can replace reuse this outside (in titiler-pgstac and titiler-stacapi)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ouch, I started doing it and found out that all three use cases are slightly different.
The difference between usage here and in titiler-stacapi is evident on BidxParams
:
get_dependency_params(
dependency=BidxParams, query_params={"bidx": [1, 2, 3]}
)
titiler-stacapi wants to receive {"index": [1,2,3]}
, I need {"bidx": [1,2,3]}
, and the key here is in alias
- code
Each project is doing a slightly different task:
- titiler-pgstac - make sure that passed params pass validation for query params
- titiler-stacapi - validate and conform passed params based on query params (or do the same thing fastapi does with incoming params)
- render extension - filter passed params to the ones that can be in query params and make sure they validate
I can write a single function to cover two use cases, but not three. I will think about it a bit more and see if something comes up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thin one function could be used for the 3 projects to do validation
and translation
.
in titiler-pgstac and titiler we want to validate
the render metadata but in titiler-stacapi we want to validate and also translate to internal key=values
.
In titiler and titiler-pgstac we return None
or at least we don't care what the return is
We could have those 2 functions in titiler.core.utils
# titiler (will be used in titiler-stacapi)
def get_dependency_params(*, dependency: Callable, query_params: Dict) -> Any:
"""Check QueryParams for Query dependency.
1. `get_dependant` is used to get the query-parameters required by the `callable`
2. we use `request_params_to_args` to construct arguments needed to call the `callable`
3. we call the `callable` and catch any errors
Important: We assume the `callable` in not a co-routine
"""
dep = get_dependant(path="", call=dependency)
if dep.query_params:
# call the dependency with the query-parameters values
query_values, _ = request_params_to_args(dep.query_params, query_params)
return dependency(**query_values)
return
# titiler-pgstac and titiler
def check_query_params(
*, dependencies: List[Callable], query_params: Union[QueryParams, Dict]
) -> None:
"""Check QueryParams for Query dependency.
1. `get_dependant` is used to get the query-parameters required by the `callable`
2. we use `request_params_to_args` to construct arguments needed to call the `callable`
3. we call the `callable` and catch any errors
Important: We assume the `callable` in not a co-routine
"""
return [
get_dependency_params(dependency, query_params=query_params)
for dependency in dependencies
]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, will move to utils.
One more note, initial implementation in render extension, does not only validate but also picks (or filters) only params that would be valid in the tiling endpoint. For example, if we have in render item of the stac {"bidx": [1, 2, 3], "other": "value"}
, now I would pass on to the tiling endpoint only {"bidx": [1, 2, 3]}
.
I prefer this approach because we are returning a URL we can guarantee will work. Otherwise, we can pass on a param that will fail on the validation, e.g. tilescale=5
.
I am bringing this up to make sure we are making the decision knowing the details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved it out to utils, but I also understood that it works a bit differently than I thought before. I will add some more comments near the relevant code to explain more.
This is great work @alekzvik 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a few comments on the decisions made.
""" | ||
dep = get_dependant(path="", call=dependency) | ||
return request_params_to_args( | ||
dep.query_params, QueryParams(urlencode(params, doseq=True)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notice the addition of QueryParams
here.
The reason is that request_params_to_args
expects to see the result of this function.
An example that helped me catch it is rescale
param - link to def.
rescale: Annotated[
Optional[List[str]],
Query(...)
]
If I pass things directly from STAC renders
block, it would not pass validation, since it is defined as [float]
To mitigate this, I first serialize params to the state that fastapi would receive them, and then deserialize how it would do it.
errors = _validate_params(render) | ||
|
||
if not errors: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
errors
here is a list of all validation errors for STAC render block.
I decided to exclude links if there are errors, as I know they are invalid and will fail if you follow the link.
An alternative is to fail louder, e.g. with HTTP 400 and error details. Let me know if you prefer it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great 🙏
Thanks a lot for the PR @alekzvik.
The overall is perfect and I'm going to merge this to be included in the next release.
The only thing that we could maybe change in the future is that by default we will put all render parameters in the tilejson.json
query parameter (e.g width=1024
or minmax_zoom
) which will be ignored by the endpoint. I think it's fine letting those in, in case someone as a custom endpoint.
What I am changing
How I did it
/renders/
lists all renders/renders/<render_id>
returns a specific render with its metadata and template links totilejson.json
andWMTS
How you can test it
Related Issues
Example responses
Those are generated based on this STAC item
Request:
http://127.0.0.1:8000/stac/renders/sir?url=https%3A%2F%2Fraw.githubusercontent.com%2Fstac-extensions%2Frender%2F65edf9bcc7dfe0a02e0191a277c1ac81d86b698e%2Fexamples%2Fitem-sentinel2.json
Response:
Request:
http://127.0.0.1:8000/stac/renders?url=https%3A%2F%2Fraw.githubusercontent.com%2Fstac-extensions%2Frender%2F65edf9bcc7dfe0a02e0191a277c1ac81d86b698e%2Fexamples%2Fitem-sentinel2.json
Response: