thb i'm not 100% sure how db defaults work but i think in the DS api create/update since routing_name is going to be optional, you'll want to see if it exists (and not "") in the request json and if so, use that value to update the db else retrieve the default (from wherever you decide) and use that value to update the db.
jeremy On Tue, Aug 15, 2017 at 9:14 AM, Peters, Rawlin <[email protected]> wrote: > How do we feel about setting a default for the column in the DB schema? > The routing name can be any arbitrary hostname (without periods) after this > support is added, and if someone doesn’t like the default we choose, they > can always provide/update their own routing_name via the UI/API. > > --Rawlin > > On 8/15/17, 8:42 AM, "Robert Butts" <[email protected]> wrote: > > +1 > > A new requirement is a breaking change, needs to be a new major > version, or > a default value that doesn't break existing usage. > > On Tue, Aug 15, 2017 at 8:40 AM, Dewayne Richardson <[email protected] > > > wrote: > > > This is a tough one because the obvious ways of breaking an API are > "URL > > format changes", "Request or Response format changes", etc. But I > think > > the more obvious way to think about the API is, do the clients have > to > > change their code? If the answer is yes, it's a breaking API change. > > > > So, really the only way around this is to "default" the new field's > value > > and make it optional, otherwise the API needs to rev. > > > > -Dew > > > > On Tue, Aug 15, 2017 at 8:09 AM, Jeremy Mitchell < > [email protected]> > > wrote: > > > > > I don't think you can add a new not null column to the ds table > because > > > that would break the DS api. For example, on DS create/update you > are > > > saying routing_name is now required, right? This is an API breaking > > change > > > that would require a new version of the API, no? > > > > > > Hence my suggestion for a default. But, yes, i forgot about the > per CDN > > > thing. > > > > > > Jeremy > > > > > > On Mon, Aug 14, 2017 at 5:25 PM, Peters, Rawlin < > > [email protected] > > > > > > > wrote: > > > > > > > Jeremy’s suggestion could work, but the param would probably be > created > > > in > > > > a TR_PROFILE per-CDN. However, that still wouldn’t fix the > visibility > > > > problem. If a CDN isn’t using the default “tr” HTTP routing name, > > > operators > > > > would still need to know that there is a new profile parameter > that > > needs > > > > updating post-upgrade but before a snap/queue. So either way > there > > needs > > > to > > > > be sufficient upgrade notes, but personally I still prefer > keeping the > > > > routing_name column non-null. > > > > > > > > That said, this is my current proposal for the DB migration > which also > > > > gets us past the upgrade issue: > > > > 1. Add a routing_name column to the DeliveryService table. > > > > 2. Update the routing_name for DNS Delivery Services to “edge”. > > > > 3. Update the routing_name of non-DNS Delivery Services to the > value > > of a > > > > temporary upgrade parameter associated with the Delivery > Service’s CDN > > > (if > > > > the upgrade parameter doesn’t exist, the routing_names will > remain > > null). > > > > 4. Update the remaining null routing_names to “tr”. > > > > 5. Make the routing_name column non-null and add a non-empty > > constraint. > > > > > > > > So these would be an operator’s pre-upgrade steps: > > > > 1. Verify if a custom http.routing.name has been configured for > > Traffic > > > > Routers in their CDNs. > > > > 2. If custom http.routing.name, do the following. Otherwise, no > > > > pre-upgrade steps needed (for per-DS routing names at least): > > > > a. create a parameter named “upgrade_http_routing_name” with > the > > > value > > > > of the target CDN’s custom http.routing.name > > > > b. associate this parameter to the TR_PROFILE belonging to > the > > target > > > > CDN > > > > c. repeat steps 2a and 2b for each CDN using a custom > > > > http.routing.name > > > > > > > > This would keep everything working the same post-upgrade as it > did > > > > pre-upgrade, and from that point on you’d be able to change a > Delivery > > > > Service’s routing name to any arbitrary hostname (without > periods). > > > > > > > > --Rawlin > > > > > > > > On 8/14/17, 4:22 PM, "Dave Neuman" <[email protected]> wrote: > > > > > > > > I don't think that solves the issue Rawlin was describing. > The > > issue > > > > that > > > > Rawlin was describing is that someone has already defined a > > different > > > > routing name in traffic_router/http.properties which is no > longer > > > > going to > > > > be used after the upgrade. The upgrade needs to somehow > know about > > > > this > > > > other routing name and use that when it creates the database > column > > > and > > > > populates it with the pre-defined defaults (edge, tr). > > > > > > > > Also, creating a global param doesn't help those with more > than 1 > > > CDN. > > > > > > > > On Mon, Aug 14, 2017 at 4:09 PM, Jeremy Mitchell < > > > > [email protected]> > > > > wrote: > > > > > > > > > Adding a temp parameter would work but I worry that > someone won't > > > > read the > > > > > upgrade documentation and forget to create this temporary > > parameter > > > > before > > > > > running the upgrade. > > > > > > > > > > Here's another option. > > > > > > > > > > Create 2 global TO parameters (http.routing.name and > > > > dns.routing.name > > > > > <http://http.routing.name/>) that default to tr and edge > > > > respectively and > > > > > make the ds.routing_name an optional field. > > > > > > > > > > in seeds.sql > > > > > > > > > > insert into parameter (name, config_file, value) values (' > > > > > http.routing.name', > > > > > 'global', 'tr') ON CONFLICT (name, config_file, value) DO > > NOTHING; > > > > > insert into parameter (name, config_file, value) values (' > > > > dns.routing.name > > > > > ', > > > > > 'global', 'edge') ON CONFLICT (name, config_file, value) DO > > > NOTHING; > > > > > > > > > > in code (warning. ugly pseudo code to follow): > > > > > > > > > > function getRoutingName(ds) { > > > > > return ds.routing_name if not null > > > > > if (ds.type = HTTP) { > > > > > return parameter.http.routing.name > > > > > } else > > > > > return parameter.dns.routing.name > > > > > } > > > > > } > > > > > > > > > > Just my 2 cents. > > > > > > > > > > On Mon, Aug 14, 2017 at 10:55 AM, Dave Neuman < > [email protected] > > > > > > > wrote: > > > > > > > > > > > Good info Rawlin. > > > > > > My vote would be for a parameter to be used during the > upgrade. > > > > We can > > > > > set > > > > > > a param called `upgrade_routing_name` or something > similiar so > > > > that it is > > > > > > obvious that it is a param used for upgrade only. We > should > > also > > > > > document > > > > > > that A) the param needs to be set before upgrade and B) > TR will > > > now > > > > > ignore > > > > > > the setting in the config file. Ideally we would remove > the > > > param > > > > from > > > > > the > > > > > > default config and the need for the param in the code. > > > > > > > > > > > > On Wed, Aug 9, 2017 at 4:28 PM, Peters, Rawlin < > > > > > [email protected]> > > > > > > wrote: > > > > > > > > > > > > > Hey all, > > > > > > > > > > > > > > I’ve dug through this a bit more, and defaulting a new > > > > > > > DeliveryService.routing_name > > > > > > > column to ‘tr’ for HTTP delivery services presents an > upgrade > > > > issue if > > > > > a > > > > > > > CDN has > > > > > > > chosen to use a custom “http.routing.name” parameter > for the > > > > Traffic > > > > > > > Routers > > > > > > > in that CDN (by editing the http.properties files of > the > > > Traffic > > > > > > Routers). > > > > > > > > > > > > > > For instance, if “http.routing.name” has been set to > “ccr”, > > > the > > > > new > > > > > > > routing name > > > > > > > “tr” will break all of the clients using the old “ccr” > > delivery > > > > service > > > > > > > URL. > > > > > > > > > > > > > > Basically we need to provide a one-time upgrade step > to allow > > > > CDNs > > > > > using > > > > > > a > > > > > > > custom > > > > > > > “http.routing.name” to default the new routing_name > column > > to > > > > that > > > > > value > > > > > > > for > > > > > > > existing HTTP delivery services. What would be the > best way > > to > > > > do this? > > > > > > > Some options > > > > > > > might be: > > > > > > > 1. Add a profile parameter to the TR_PROFILE for that > CDN. On > > > > upgrade, > > > > > > > read that > > > > > > > parameter and use it to update the routing_name for > > > existing > > > > HTTP > > > > > > > delivery services. > > > > > > > After upgrade, you can safely remove the profile > > parameter. > > > > > > > 2. Let the upgrade automatically default the > routing_name to > > > > ‘tr’ or > > > > > > > ‘edge’. After > > > > > > > upgrading, manually update each HTTP delivery > service to > > > use > > > > the > > > > > > > current > > > > > > > “http.routing.name” in use (we could provide an > API > > > > endpoint to > > > > > > “bulk > > > > > > > update” the > > > > > > > routing names for all HTTP delivery services in a > CDN). > > > > > > > > > > > > > > Note this is not an exhaustive list, this is a just a > couple > > > > options > > > > > that > > > > > > > have come up in > > > > > > > discussion so far. Feel free to add any more ideas to > this > > > list. > > > > > > > > > > > > > > After the upgrade has been completed, the “ > http.routing.name > > ” > > > > > parameter > > > > > > > in the > > > > > > > Traffic Router’s “http.properties” file will be > ignored (same > > > > with the > > > > > “ > > > > > > > dns.routing.name” > > > > > > > parameter in “dns.properties” which I’m not sure can > even be > > > > changed > > > > > > > safely today). > > > > > > > > > > > > > > Thoughts? > > > > > > > > > > > > > > --Rawlin > > > > > > > > > > > > > > On 8/4/17, 10:19 AM, "Peters, Rawlin" < > > > [email protected] > > > > > > > > > > wrote: > > > > > > > > > > > > > > @Dave @JvD > > > > > > > > > > > > > > Thanks for the feedback. I think I can get on > board with > > > > defaulting > > > > > > > the DS columns to ‘edge’ and ‘tr’. I was thinking the > CDN > > > > columns might > > > > > > be > > > > > > > useful if the user just wants to set it CDN-wide and > not > > > > individually > > > > > on > > > > > > > each DS, but I guess if we default it as part of the > upgrade > > > > migration, > > > > > > we > > > > > > > should also provide an API endpoint to set the routing > names > > on > > > > all > > > > > DSes > > > > > > in > > > > > > > a CDN to a single value, thus still providing a > “per-CDN” > > > option. > > > > > Would a > > > > > > > “bulk” update also be useful, in case a user wants to > update > > a > > > > handful > > > > > of > > > > > > > DSes to the same routing names at once? > > > > > > > > > > > > > > @JvD Re: TR_PROFILE vs. DS_PROFILE > > > > > > > The default would come from a TR_PROFILE linked to > the > > CDN, > > > > and the > > > > > > > override would come from a DS_PROFILE linked to that > specific > > > > DS. I > > > > > > looked > > > > > > > into those options to cover all my bases, but I think > adding > > > > columns to > > > > > > at > > > > > > > least the DS table and not touching profiles at all is > the > > > better > > > > > option. > > > > > > > > > > > > > > -Rawlin > > > > > > > > > > > > > > On 8/4/17, 8:04 AM, "Jan van Doorn" < > [email protected]> > > > wrote: > > > > > > > > > > > > > > Agree with Dave on > > > > > > > > > > > > > > [*DN] we should default the database column to > "edge" > > > > for DNS > > > > > and > > > > > > > "tr" for* > > > > > > > *http. Then we don't have to do the null > check.* > > > > > > > > > > > > > > If we do that, we can make the columns > mandatory, and > > > it > > > > makes > > > > > > > sense > > > > > > > they're not in the DS_PROFILE. Also makes it > so we > > > don't > > > > have > > > > > to > > > > > > > have a CDN > > > > > > > wide setting. (and Rawlin, I think you mean to > say > > > > DS_PROFILE > > > > > > > rather than > > > > > > > TR_PROFILE type to add the param to if we > chose to do > > > > that?? Or > > > > > > > was it the > > > > > > > default that goes into TR_PROFILE and the > override > > into > > > > > > > DS_PROFILE?). > > > > > > > In any case - if we make the columns NOT NULL > and > > > > default them > > > > > to > > > > > > > "tr" and > > > > > > > "edge", I'm +1 on columns in the > deliveryservice > > table. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > JvD > > > > > > > > > > > > > > On Fri, Aug 4, 2017 at 7:12 AM Eric Friedrich > > > (efriedri) > > > > < > > > > > > > [email protected]> > > > > > > > wrote: > > > > > > > > > > > > > > > Hey Rawlin- > > > > > > > > Zhilin has also been working on a very > similar > > > > feature > > > > > which > > > > > > > was > > > > > > > > proposed on this mailer last month: > > > > > > > > https://lists.apache.org/thread.html/ > > > > > > > 51d7ed1ae65a3697c39edd00236e6f > 3897da37ef5b24ac452a17cabb@% > > > > > > > 3Cdev.trafficcontrol.apache.org%3E > > > > > > > > < > > > > > > > > https://lists.apache.org/thread.html/ > > > > > > > 51d7ed1ae65a3697c39edd00236e6f > 3897da37ef5b24ac452a17cabb@ > > > > > > > > <dev.trafficcontrol.apache.org>> > > > > > > > > > > > > > > > > Can you please work him to ensure we don’t > > duplicate > > > > work and > > > > > > > that if both > > > > > > > > solutions are needed they will work together? > > > > > > > > > > > > > > > > On Aug 3, 2017, at 3:57 PM, Peters, Rawlin < > > > > > > > [email protected] > > > > > > > > <mailto:[email protected]>> wrote: > > > > > > > > > > > > > > > > Sorry, Outlook converted my numbered list > poorly. > > > I’ve > > > > > > corrected > > > > > > > the > > > > > > > > numbering (items 1-3) below. > > > > > > > > > > > > > > > > On 8/3/17, 1:52 PM, "Peters, Rawlin" < > > > > > > [email protected]< > > > > > > > mailto: > > > > > > > > [email protected]>> wrote: > > > > > > > > > > > > > > > > Hello All, > > > > > > > > > > > > > > > > I’ve been working on adding support for > > > configurable > > > > > per-CDN > > > > > > > and > > > > > > > > per-DeliveryService routing names [1] (what > are > > > > currently > > > > > > > > hardcoded/defaulted to ‘edge’ and ‘tr’ for > DNS and > > > HTTP > > > > > > Delivery > > > > > > > Services, > > > > > > > > respectively), and I have a few things to > propose. > > > > > > > > > > > > > > > > > > > > > > > > 1. Add a column to the CDN table for > the DNS > > > and > > > > HTTP > > > > > > > routing names. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I’ve currently been working off the > assumption > > > that > > > > > per-CDN > > > > > > > routing > > > > > > > > names would be configurable by adding ‘ > > > > http.routing.name’ > > > > > and > > > > > > ‘ > > > > > > > > dns.routing.name’ parameters to a profile > of type > > > > TR_PROFILE > > > > > > > using the > > > > > > > > ‘CRConfig.json’ config file. To me this > seems like > > > bad > > > > UX > > > > > > > because the user > > > > > > > > has to click through multiple steps and fill > in > > > > multiple > > > > > fields > > > > > > > in the UI > > > > > > > > just to change the CDN’s routing names. It > also > > > > requires > > > > > > joining > > > > > > > a few > > > > > > > > different tables in the DB just to find the > > > parameters > > > > > per-CDN. > > > > > > > For that > > > > > > > > reason, I think it would be better if > > > > ‘dns_routing_name’ and > > > > > > > > ‘http_routing_name’ were added as columns of > the > > > ‘cdn’ > > > > table, > > > > > > > and changing > > > > > > > > them via the UI would follow the same > process as > > > > choosing the > > > > > > > CDN’s domain > > > > > > > > name. Because the routing names would be the > > CDN-wide > > > > > defaults, > > > > > > > the ‘Edit > > > > > > > > CDN’ window feels like the most natural > place to > > put > > > > it. > > > > > > > > > > > > > > > > > > > > > > > > 2. Values for per-DeliveryService > routing > > names > > > > could > > > > > > live > > > > > > > in one of > > > > > > > > a couple different areas: > > > > > > > > * New columns in the > delivery_service > > table > > > > > > > > * Parameters in a DS Profile > > > > > > > > > > > > > > > > As the developer, my vote would be for > Option A > > > > because it > > > > > > > seems like > > > > > > > > it would lead to cleaner code in Traffic Ops > > because > > > > the > > > > > > routing > > > > > > > names > > > > > > > > would be readily-available when handling a > > > > DeliveryService. > > > > > You > > > > > > > wouldn’t > > > > > > > > have to also fetch its profile then dig > through it > > to > > > > find > > > > > the > > > > > > > routing > > > > > > > > names. A downside could be that adding > columns to > > an > > > > > > > already-overcrowded > > > > > > > > table isn’t ideal. > > > > > > > > > > > > > > > > Option B is less appealing to me but > might have > > > some > > > > > > > advantages such as > > > > > > > > keeping the number of columns down in the > > > > DeliveryService > > > > > > table. > > > > > > > However, > > > > > > > > DS Profiles currently seem to be geared more > > towards > > > > the > > > > > > > Multi-site Origin > > > > > > > > feature in generating specific ATS > configuration > > > > > > (parent.config) > > > > > > > and less > > > > > > > > towards a “junk drawer for optional config”. > As the > > > > routing > > > > > > > names would > > > > > > > > affect the entire DS and multiple config > files, it > > > > doesn’t > > > > > seem > > > > > > > right to > > > > > > > > have it as a profile parameter using > > ‘CRConfig.json’ > > > > as the > > > > > > > config file. I > > > > > > > > wasn’t around when DS Profiles were > introduced, so > > if > > > > you are > > > > > > > more familiar > > > > > > > > with their purpose/origin and think this is > a good > > > fit > > > > for > > > > > > them, > > > > > > > I’d like > > > > > > > > to hear your advice. > > > > > > > > > > > > > > > > > > > > > > > > 3. If per-DeliveryService routing > names are > > not > > > > set > > > > > > > explicitly for a > > > > > > > > DS (i.e. the column is null), then the DS > will use > > > the > > > > > per-CDN > > > > > > > routing > > > > > > > > names as a default. If the per-CDN routing > names > > are > > > > unset, > > > > > > they > > > > > > > will > > > > > > > > default to the current values of ‘edge’ and > ‘tr’. > > So > > > > the > > > > > lookup > > > > > > > hierarchy > > > > > > > > would be DS.routing_names -> > CDN.routing_names -> > > > > default > > > > > > > ‘edge/tr’. > > > > > > > > > > > > > > > > I’d like to know what you think of these > > > proposals, > > > > and > > > > > any > > > > > > > > advice/feedback is welcome. > > > > > > > > > > > > > > > > Best regards, > > > > > > > > Rawlin > > > > > > > > > > > > > > > > [1] https://issues.apache.org/ > > jira/browse/TC-287 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
