basically what i'm saying is if you are going to keep routing_name as a non null column you are going to have to handle it's potential absence in the api layer....i think...
jeremy On Tue, Aug 15, 2017 at 10:02 AM, Jeremy Mitchell <[email protected]> wrote: > 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 >> > > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > > >> > > > >> > > >> > >> >> >> >
