WIP connect titiler for custom search tiling #97

kylebarron · 2021-02-24T01:12:10Z

This is a draft PR to connect titiler with STAC api's search endpoint.

Notes:

This only cares about the tile endpoint of the MosaicTilerFactory; I haven't tested anything else.
This adds an extension file, but doesn't add it to the main app by default because that would require adding more dependencies: cogeo_mosaic, morecantile, rio_tiler. (Though I guess the STAC API mounts titiler by default already, so I guess this isn't actually adding any more dependencies?)

kylebarron · 2021-02-24T01:16:39Z

stac_api/api/extensions/tiles_search.py

+    ids: Optional[str] = Query(None)
+    datetime: Optional[str] = Query(None)
+    limit: Optional[int] = Query(10)
+    query: Optional[str] = Query(None)
+    token: Optional[str] = Query(None)
+    fields: Optional[str] = Query(None)
+    sortby: Optional[str] = Query(None)


todo: add title and description for each of these, so it shows up in openapi

kylebarron · 2021-02-24T01:17:41Z

stac_api/api/extensions/tiles_search.py

+    Custom extension for sending query results to a titiler DynamicSTAC endpoint
+    """
+
+    client: CoreCrudClient = attr.ib(default=attr.Factory(CoreCrudClient))


This uses the base CoreCrudClient as all it needs is the client.get_search method. I figured it wasn't necessary to subclass CoreCrudClient like the TilesClient does

https://github.com/arturo-ai/arturo-stac-api/blob/7ca0c8790ac64bbd359e88933ef94f25ece5b383/stac_api/clients/tiles/ogc.py#L17-L41

makes sense. TilesClient arguably shouldn't subclass CoreCrudClient either. I think composition is a better pattern here.

kylebarron · 2021-02-24T01:18:46Z

stac_api/api/extensions/tiles_search.py

+        """
+        dynamic_stac_endpoint = MosaicTilerFactory(
+            reader=DynamicStacBackend,
+            reader_options={"client": self.client},


What's the best way to pass the client to the MosaicTilerFactory? I need to access the client in the MosaicBackend.get_assets method, but it seems like it shouldn't be on the reader_options. Maybe it's best to subclass the DynamicStacBackend within this extension so you can attach the client there...

but it seems like it shouldn't be on the reader_options.

I think it's ok to use the reader_options here and just pass a backend_options dict within.

reader_options={"backend_options":{"client": self.client}},

that's how we do for STAC https://github.com/developmentseed/cogeo-mosaic/blob/master/cogeo_mosaic/backends/stac.py#L76

I agree it's not perfect

Wait, in fact you are already creating a custom Mosaic Backend so you can still use reader_options={"client": self.client} and make client a required parameter for your DynamicStacBackend backend

kylebarron · 2021-02-24T01:22:13Z

stac_api/api/extensions/tiles_search.py

+
+        print("feature_collection")
+        print(feature_collection)
+        return [default_stac_accessor(f) for f in feature_collection["features"]]


I don't fully understand this... seems like here I'm returning the id of each stac item, rather than the stac items themselves... Seems like it would be better to return feature_collection['features'] and then use the resulting STAC dicts directly.

kylebarron · 2021-02-24T01:23:48Z

stac_api/api/extensions/tiles_search.py

+
+
+@attr.s
+class FakeRequest:


This is to mock the request param passed through by default here https://github.com/arturo-ai/arturo-stac-api/blob/7ca0c8790ac64bbd359e88933ef94f25ece5b383/stac_api/api/routes.py#L58-L66

A way to get around this is to inject a dependency which itself returns a request object. This works because dependencies always have access to the request:

from starlette.requests import Request def request_dependency(request: Request): return request

kylebarron · 2021-02-24T02:19:08Z

stac_api/api/extensions/tiles_search.py

+            router_prefix="test-dynamic-search",
+        )
+
+        app.include_router(
+            dynamic_stac_endpoint.router, prefix="/dynamic-stac", tags=["Dynamic STAC"]


It seems unnecessary to have both of these router prefixes, and it looks like /dynamic-stac overrides test-dynamic-search anyways. Also, open to other prefix names.

it is unnecessary, I think i'd rather include the prefix on the call to include_router.

geospatial-jeff · 2021-02-24T02:46:00Z

To summarize our slack convo:

The "all-in-one" approach where we overload a single endpoint to POST /search and return tiles is simpler to implement but doesn't align that well with the way that others typically implement STAC + tiling. The typical approach is to add a new link which references a tile layer for that particular resource.

{
  "title": "tile-layer",
  "type": "image/jpeg",
  "href": "http://tile-layer/{z}/{x}/{y}.jpg",
  "templated": true,
  "rel": "tiles",
}

In my mind this has two primary benefits:

Its more canonical "STAC", where additional info/metadata/functionality etc. is exposed as a link on the particular resource.
It lets us hook into specifications like OGC API Tiles which has a very sensible and small core that provides nice interoperable way of linking STAC to tiles. This is actually how the existing titiler extensions works, albeit only on a single item so its a much easier problem.

My dream for STAC + tiling is exposing tiles links at the Item, ItemCollection, and Catalog levels; where item collection and catalog links are mosaicing the appropriate items. The actual implementation of this is difficult though (imo order of increasing difficulty is Item -> ItemCollection -> Catalog). And the "all-in-one" approach provides a much more constrained example that I think we should move forward with because it should give us valuable information about how to implement STAC + tiling as described above.

In particular I think breaking up this repo into appropriate parts (stac-api / db / extension - ref @vincentsarago) will clear up separation of concerns and make thinking through the implementation a little easier.

kylebarron · 2021-02-24T03:21:14Z

In general I think it makes sense to put tiling rel links on Item, ItemCollection, and Catalog, however it seems you hit unnecessary back-and-forth HTTP requests if the client needs to first create an ItemCollection via the /search endpoint, then post those stac items back to tile them.

exposing tiles links at the Item, ItemCollection, and Catalog levels; where item collection and catalog links are mosaicing the appropriate items

I think this makes sense, but in my mind this PR is essentially intending to serve the "tile a Catalog" problem. For almost any Catalog, like Landsat 8/Sentinel 2, you have way too many images per relevant tile, so you still need a search within the catalog.

kylebarron · 2021-02-24T03:26:56Z

The typical approach is to add a new link which references a tile layer for that particular resource.
{
  "title": "tile-layer",
  "type": "image/jpeg",
  "href": "http://tile-layer/{z}/{x}/{y}.jpg",
  "templated": true,
  "rel": "tiles",
}

By the way this isn't necessarily in conflict with a /search/tiles approach. A /search endpoint that returns an ItemCollection could have a tiles link back to /search/tiles in order to tile the contained features. And it could set /search params like limit and token so that those exact same features would be used. In effect it would mostly trade another SELECT against the db for avoiding POSTing the ItemCollection back to a tiler.

geospatial-jeff · 2021-02-24T03:34:14Z

I think this makes sense, but in my mind this PR is essentially intending to serve the "tile a Catalog" problem.

Makes sense to me. Catalog tiling should happen through /search?collections=collection_name to minimize search space. Catalog tiling is quite similar to ItemCollection tiling in this way, at the end of the day it all comes down to creating a mosaic from List[Item].

By the way this isn't necessarily in conflict with a /search/tiles approach. A /search endpoint that returns an ItemCollection could have a tiles link back to /search/tiles in order to tile the contained features.

This is a great approach to make this more consumable, let's roll with it.

vincentsarago · 2021-07-22T09:28:13Z

FYI, I've started something similar over https://github.com/stac-utils/titiler-pgstac

Note: This is not an extension and doesn't add new endpoint to a stac-api but create a new standalone app

lossyrob · 2021-07-25T15:24:51Z

What's the status of this PR? IMO stac-fastapi shouldn't host its own tiler - the titiler-pgstac is a good example of standing up a tiler that uses STAC but a separate service that can be deployed independently. In our implementation of stac-fastapi, we override the pgstac backend core client in order to iterate over collections/items and inject tiler links where appropriate. I think this approach allows stac-fastapi to concentrate on just the STAC API endpoints, but allows customization by users if they want to add titiler or other tiling capability. I could see work done to generalize and implement a link injection mechanism so that other libraries can write hooks into the STAC objects returned by stac-fastapi so that the link/asset injection logic can live in that external project. But I don't think we should create tilers directly in stac-fastapi.

geospatial-jeff · 2021-07-26T02:48:16Z

I agree the code for such an integration should live somewhere else. Closing this PR.

I could see work done to generalize and implement a link injection mechanism so that other libraries can write hooks into the STAC objects returned by stac-fastapi so that the link/asset injection logic can live in that external project.

This would be a good feature.

WIP connect titiler for custom search tiling

f774a83

kylebarron commented Feb 24, 2021

View reviewed changes

Pass stac items to STACReader directly

eb035e7

kylebarron commented Feb 24, 2021

View reviewed changes

geospatial-jeff mentioned this pull request Feb 24, 2021

TilesClient shouldn't subclass CoreCrudClient #99

Closed

geospatial-jeff closed this Jul 26, 2021

geospatial-jeff mentioned this pull request Jul 26, 2021

Add a way for users to inject their own links into API responses #191

Open



		@attr.s
		class FakeRequest:

WIP connect titiler for custom search tiling #97

WIP connect titiler for custom search tiling #97

Uh oh!

Conversation

kylebarron commented Feb 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kylebarron Feb 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geospatial-jeff Feb 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geospatial-jeff commented Feb 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kylebarron commented Feb 24, 2021

Uh oh!

kylebarron commented Feb 24, 2021

Uh oh!

geospatial-jeff commented Feb 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vincentsarago commented Jul 22, 2021

Uh oh!

lossyrob commented Jul 25, 2021

Uh oh!

geospatial-jeff commented Jul 26, 2021

Uh oh!

Uh oh!

kylebarron commented Feb 24, 2021 •

edited

Loading

kylebarron Feb 24, 2021 •

edited

Loading

geospatial-jeff Feb 24, 2021 •

edited

Loading

geospatial-jeff commented Feb 24, 2021 •

edited

Loading

geospatial-jeff commented Feb 24, 2021 •

edited

Loading