search

go-spatial / tegola

Tegola is a Mapbox Vector Tile server written in Go
http://tegola.io/
MIT License
1.28k stars 196 forks source link

handle gpkg GEOMETRY as unknown geometry #845

Closed roelarents closed 2 years ago

roelarents commented 2 years ago

I would like geopakcage GEOMETRY type to not result in a startup error , but instead to be handled as an unknown type.

The layer type is used for rendering the capabilities json and styles. In the capabilities this layer will be set to unknown. A style will not be generated and a warning will be logged.

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build e64c6eaf2-PR-845

Totals Coverage Status
Change from base Build 58e3eac8d: 0.009%
Covered Lines: 5610
Relevant Lines: 12380
💛 - Coveralls
gdey commented 2 years ago

@roelarents Thank you for your contribution!

I'm working through this and thinking about it. Just wanted to let you know that I have looked at it, and trying to understand what is trying to be accomplished and what are the possible options.

gdey commented 2 years ago

@roelarents I'm not coming up with a solution I'm happy with, so I want to outline my thinking, and see if we can work through things.

I don't like returning error values as a a geometry value (https://github.com/go-spatial/tegola/blob/b1411adcb9e4e42b30657ebdc8bc92f5a40c51d5/provider/gpkg/gpkg.go#L264-L265) even if it's only limited to the gpkg provider.

I could see where someone would want to get the error if the mapping geo type is not supported. I, almost, think it makes sense to add a switch here (https://github.com/go-spatial/tegola/blob/b1411adcb9e4e42b30657ebdc8bc92f5a40c51d5/provider/gpkg/gpkg_register.go#L194-L198) to check the error type returned and based on a flag set from the config ( https://github.com/go-spatial/tegola/blob/b1411adcb9e4e42b30657ebdc8bc92f5a40c51d5/provider/gpkg/gpkg_register.go#L222) and passed to the function (https://github.com/go-spatial/tegola/blob/b1411adcb9e4e42b30657ebdc8bc92f5a40c51d5/provider/gpkg/gpkg_register.go#L244) we could support both needs.

But I am unsure if this is the best way forward. I just know that to me passing an error value for a geometry smells.

thank you,

Gautam.

roelarents commented 2 years ago

I understand your objections and I agree. Maybe a new type could be added to go-spatial/geom, called a "MultiGeometry" or "MixedGeometry" or "AnyGeometry" or something. I also considered adding a type in the gpkg package and returning that.

For now, I've made a new try by just returning nil (without an error). Which still has the desired effect on capabilities json and styles.

gdey commented 2 years ago

@roelarents if you could please rebase this to master, I will get it merged in and ready for the next release.

roelarents commented 2 years ago

@gdey done and thanks