replies inline On Thu, Jul 5, 2018 at 9:24 AM Robert Butts <[email protected]> wrote: > > +1 on listing them as an array. JSON arrays are defined to be ordered, so > that lets us use the array order as the lookup order, making it easy to add > that as a feature in the future. > > -1 on array types in the database. It's unnecessary denormalization, and > will be harder to deal with in the future. We can do the same thing with a > normal join table: > > cg | localization | priority > ---------------------------- > foo | coverage_zone | 1 > foo | maxmind | 2 > foo | deep | 0 > bar | coverage_zone | 0 > > Which maps to the config `"permissions": [ "deep", "coverage-zone", > "geo"],` and also addresses your points on future types (we just add a new > enum value to the localization type). The additional `priority` column lets > us order it in the JSON array via SELECT ORDER BY.
Just to be clear, the ordering of "cachegroup permissions" doesn't matter because it's really a Set not an Array. If we wanted to prioritize different localization methods, I'd think that would have to be done on a per-DS basis (because the cachegroup is chosen *after* the localization methods of the DS), and we'd have to overhaul TR localization into a type of prioritized pipeline of localization methods per-DS. That would be pretty cool IMO, but I can't see localization priorities being an attribute of the cachegroup itself and think they should be left off the cachegroup-localization table. > > +1 on rethinking the cachegroup fallback API, too. We should have more join > tables, and better data normalization, but it's painful to use HTTP/JSON > APIs like that. Our APIs will be much easier to use, if we can stop > exposing join tables to the API altogether (e.g. cachegroup_fallback, > deliveryservice_regex), and only insert them into their proper objects > (cachegroup, deliveryservice), and then manipulate them within the object > via POST/PUT/PATCH on the larger object. @VijayAnand is hopefully following along :). I think we got the DB schema right for cachegroup_fallbacks, but I think if we got rid of the cachegroup_fallbacks API and just did it within the existing cachegroup API with an array, it would make using the API to configure fallbacks much easier. It would probably make the Traffic Portal implementation much more straightforward too, not requiring entirely new views.
