CartoDB / Windshaft-cartodb

Windshaft tailored for CARTO
BSD 3-Clause "New" or "Revised" License
72 stars 59 forks source link

Should overviews metadata retrieval affect layergroup instantiation? #483

Open rochoa opened 8 years ago

rochoa commented 8 years ago

As per: https://github.com/CartoDB/Windshaft-cartodb/blob/5eda4888edf665eeb060647ade2c734f55cbf914/lib/cartodb/models/mapconfig/adapter/mapconfig-overviews-adapter.js#L70-L76

If for some reason the metadata retrieval for overviews or filters associated to them fails, the layergroup instantiation will fail. IMO we should continue, or at least only fail if there is some metadata related to overviews and we have failed when computing the filters.

What do you think?

jgoizueta commented 8 years ago

I think that in case of error we should continue without modifying the layer (no query_rewrite_data added), so the layer will be processed as if no overviews were involved.

And I think this should be the case for any error related to overviews or filters, so if overview metadata is found but there's some problem fetching the filters we should continue and fall back to the no overviews situation (as we cannot use the overviews properly without the filters information). Ignoring the overviews rather than failing in this case can be a better option 'cause there might be some unsupported cases (e.g. non trivial analyses) that should not fail just because overviews cannot be used.

jgoizueta commented 8 years ago

We could completely avoid letting errors get out of overviewsMetadataApi.getOverviewsMetadata, just have valid overviews/filter data augmentation or nothing at all.

In some cases, if the error is unrelated to overviews (e.g. CDB_QueryTablesText failing) it will provoke another fail elsewhere and be handled there.