I can buy that. I'll leave the NOT NULL on routing_name alone. And yes, I agree, ANY_MAP seems to be quite an oddball.
On Wed, Nov 15, 2017 at 1:10 PM, Rawlin Peters <[email protected]> wrote: > I'm not a big fan of the ANY_MAP type DeliveryService. This is from > the documentation (basically the only information about this type): > > """ > ANY_MAP is not known to Traffic Router. For this deliveryservice, the > “Raw remap text” field in the input form will be used as the remap > line on the cache. > """ > > That statement is false because if an ANY_MAP DS is set to active, it > will make it into the CRConfig and be treated as an HTTP DS by the > Traffic Router [1]. But when it's inactive, it's "Raw remap line" > still gets deployed to its assigned caches remap.config [2]. I think > the only problem it was trying to solve was adding special lines to > remap.config for certain sets of caches, so I believe any TR-specific > fields of a DS don't apply to the ANY_MAP type, i.e.: > > - active (because it should always be false yet still ends up in > remap.config?) > - ccr_dns_ttl > - geo_limit > - geo_limit_contries > - geo_provider > - geolimit_redirect_url > - regional_geo_blocking > - tr_request_headers > > For the routing_name column, if we remove the NOT NULL constraint, > then we still have the problem of interpreting NULL as some string, > and it's much easier to keep it NOT NULL and make sure a default gets > inserted in the API layer rather than sprinkle an "if undefined use > 'blah'" guard all over the codebase. So, if we removed the NOT NULL > constraint, I think we'd still have to keep enforcing that constraint > in the API layer so that no null values end up in the routing_name > column. > > - Rawlin > > [1] https://github.com/apache/incubator-trafficcontrol/issues/1121 > [2] https://github.com/apache/incubator-trafficcontrol/issues/905 > > > On Wed, Nov 15, 2017 at 11:07 AM, Jeremy Mitchell <[email protected]> > wrote: > > I agree with you 100%, rob butts but i'd rather not tackle that big of a > > database refactor at the moment. :) > > > > In the short term, I'd like to remove the NOT NULL database constraints > > from the columns that don't pertain to ALL delivery services. Sadly, > > overloading the ds table removed the ability to perform validation at the > > database level which I think is actually pretty important but I guess we > > can use the API/UI level validation for the time being. > > > > Jeremy > > > > > > > > On Wed, Nov 15, 2017 at 11:00 AM, Robert Butts <[email protected] > > > > wrote: > > > >> If the field only applies to some DS types, it should be in its own > table. > >> Then it can be NOT NULL in that table, ideally with a constraint > requiring > >> the DS to be a type it applies to. You wanted to reduce the columns in > the > >> `deliveryservice` table, right? Here's your chance :) > >> > >> On Wed, Nov 15, 2017 at 10:56 AM, Jeremy Mitchell < > [email protected]> > >> wrote: > >> > >> > From what I can tell, there are 4 flavors of delivery services: > >> > > >> > 1. DNS* > >> > 2. HTTP* > >> > 3. STEERING* > >> > 4. ANY_MAP > >> > > >> > And the properties associated with each flavor vary so I put together > a > >> > spreadsheet to show which properties pertain to each flavor: > >> > > >> > https://docs.google.com/spreadsheets/d/1rb4WzP1FicumyU1z6o_ > >> > wasxD0ih1HoSH3bT-IA1GSlw/edit?usp=sharing > >> > > >> > First things first, anything look wrong there? Have I misrepresented / > >> > misunderstood anything? > >> > > >> > Things that jump out at me: > >> > > >> > 1. dscp CANNOT be null yet it only applies to 2/4 of the delivery > service > >> > types (DNS* and HTTP*) > >> > 2. regional_geo_blocking CANNOT be null yet it only applies to 2/4 of > the > >> > delivery service types (HTTP* and ANY_MAP) > >> > 3. routing_name CANNOT be null yet it only applies to 3/4 of the > delivery > >> > service types (DNS*, HTTP* and STEERING*) > >> > 4. What's up with ssl_key_version? Is this even used? I don't see it > in > >> the > >> > TO UI or the TP UI. > >> > > >> > Suggestions: > >> > > >> > 1. we remove the NOT NULL constraint from dscp, regional_geo_blocking > and > >> > routing_name since they don't apply to ALL delivery services and > rather > >> we > >> > enforce those fields in the API > >> > 2. we get rid of the ssl_key_version column unless someone knows if > it's > >> > used and how > >> > > >> > Thoughts? Concerns? > >> > > >> > Jeremy > >> > > >> >
