Re: Delivery Service Properties

2017-11-15 Thread Jeremy Mitchell
I've also created an issue (
https://github.com/apache/incubator-trafficcontrol/issues/1543) and
subsequent PR (https://github.com/apache/incubator-trafficcontrol/pull/1548)
to address the different requirements of each type of DS.

On Wed, Nov 15, 2017 at 3:10 PM, Jeremy Mitchell 
wrote:

> after thinking about this a bit  more, I think I will just leave dscp,
> regional_geo_blocking and routing_name as NOT NULL in the database. it's
> probably easier to work around those requirements than removing the
> constraint and having to deal with the fallout of that.
>
> jeremy
>
> On Wed, Nov 15, 2017 at 2:02 PM, Jeremy Mitchell 
> wrote:
>
>> 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 
>> 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 
>>> 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 <
>>> robert.o.bu...@gmail.com>
>>> > 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 <
>>> mitchell...@apache.org>
>>> >> 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 

Re: Delivery Service Properties

2017-11-15 Thread Rawlin Peters
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  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 
> 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 
>> 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
>> >
>>


Re: Delivery Service Properties

2017-11-15 Thread Robert Butts
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 
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
>


Delivery Service Properties

2017-11-15 Thread Jeremy Mitchell
>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


Re: [VOTE] Release Apache Traffic Control (incubating) 2.1.0-RC2

2017-11-15 Thread Hank Beatty

Yes, of course. How does Friday 11/17 or Monday 20/17 sound?

On 11/14/2017 04:09 PM, Dave Neuman wrote:

Hey Hank,
It looks like you have the votes you need to pass, but can you leave it
open a little longer for those of us that haven't gotten a chance to test
yet?

Thanks,
Dave

On Tue, Nov 14, 2017 at 12:47 PM, Eric Friedrich (efriedri) <
efrie...@cisco.com> wrote:


I’m +1 as well

Checked out:
- signatures/checksums (Hank is your key signed yet?)
- licenses
- build

—Eric



On Nov 14, 2017, at 2:36 PM, Nir Sopher  wrote:

+1
We were able to build traffic-control, install and connect OPs,

Portal-V2,

Monitor (Golang), Router and Stats.
Also got a redirect.
Note that I missed the last commit ("Change cdn.name to cdn.domain_name

in

DeliveryServiceInfoForDomainList"), but as far as I see it could not

break

the installation.
Nir

On Tue, Nov 14, 2017 at 8:10 PM, Eric Friedrich (efriedri) <
efrie...@cisco.com> wrote:


Thanks Matt-
  I found another LEGAL ticket (https://issues.apache.org/
jira/browse/LEGAL-330) based on a Google version of the PATENTS file

this

time.

Looks like things are OK to use then.

—Eric

On Nov 14, 2017, at 12:50 PM, Mills, Matthew > wrote:

FYI, Go itself has the same file

https://github.com/golang/go/blob/master/PATENTS


On 11/14/17, 10:36:43 AM, "Eric Friedrich (efriedri)" <

efrie...@cisco.com>

wrote:

   I’ve been going through licensing for the 2.1 release and found this
file:
   ./traffic_stats/vendor/golang.org/x/net/PATENTS

   This looks like it places some of the same restrictions that caused

the

whole Facebook React.js and rocksDb controversy a few months ago.
   Fun reading here: https://issues.apache.org/jira/browse/LEGAL-303
   There’s some in depth discussion of the detailed Facebook license, I
can’t even begin to speculate how that compares to this Google

conditional

patent grant.

   We can see what the IPMC/Legal thinks or maybe just remove the code?

   —Eric












Re: [VOTE] Release Apache Traffic Control (incubating) 2.1.0-RC2

2017-11-15 Thread Hank Beatty
Yes. Steve Malenfant signed it. Unfortunately, it looks like he is not in the "circle of trust" in the KEYS file. Does 
that matter?


On 11/14/2017 02:47 PM, Eric Friedrich (efriedri) wrote:

I’m +1 as well

Checked out:
- signatures/checksums (Hank is your key signed yet?)
- licenses
- build

—Eric



On Nov 14, 2017, at 2:36 PM, Nir Sopher  wrote:

+1
We were able to build traffic-control, install and connect OPs, Portal-V2,
Monitor (Golang), Router and Stats.
Also got a redirect.
Note that I missed the last commit ("Change cdn.name to cdn.domain_name in
DeliveryServiceInfoForDomainList"), but as far as I see it could not break
the installation.
Nir

On Tue, Nov 14, 2017 at 8:10 PM, Eric Friedrich (efriedri) <
efrie...@cisco.com> wrote:


Thanks Matt-
  I found another LEGAL ticket (https://issues.apache.org/
jira/browse/LEGAL-330) based on a Google version of the PATENTS file this
time.

Looks like things are OK to use then.

—Eric

On Nov 14, 2017, at 12:50 PM, Mills, Matthew > wrote:

FYI, Go itself has the same file

https://github.com/golang/go/blob/master/PATENTS


On 11/14/17, 10:36:43 AM, "Eric Friedrich (efriedri)" 
wrote:

   I’ve been going through licensing for the 2.1 release and found this
file:
   ./traffic_stats/vendor/golang.org/x/net/PATENTS

   This looks like it places some of the same restrictions that caused the
whole Facebook React.js and rocksDb controversy a few months ago.
   Fun reading here: https://issues.apache.org/jira/browse/LEGAL-303
   There’s some in depth discussion of the detailed Facebook license, I
can’t even begin to speculate how that compares to this Google conditional
patent grant.

   We can see what the IPMC/Legal thinks or maybe just remove the code?

   —Eric









Re: [VOTE] Release Apache Traffic Control (incubating) 2.1.0-RC2

2017-11-15 Thread Steve Malenfant
+1 here as well.

On Tue, Nov 14, 2017 at 4:09 PM, Dave Neuman  wrote:

> Hey Hank,
> It looks like you have the votes you need to pass, but can you leave it
> open a little longer for those of us that haven't gotten a chance to test
> yet?
>
> Thanks,
> Dave
>
> On Tue, Nov 14, 2017 at 12:47 PM, Eric Friedrich (efriedri) <
> efrie...@cisco.com> wrote:
>
> > I’m +1 as well
> >
> > Checked out:
> > - signatures/checksums (Hank is your key signed yet?)
> > - licenses
> > - build
> >
> > —Eric
> >
> >
> > > On Nov 14, 2017, at 2:36 PM, Nir Sopher  wrote:
> > >
> > > +1
> > > We were able to build traffic-control, install and connect OPs,
> > Portal-V2,
> > > Monitor (Golang), Router and Stats.
> > > Also got a redirect.
> > > Note that I missed the last commit ("Change cdn.name to
> cdn.domain_name
> > in
> > > DeliveryServiceInfoForDomainList"), but as far as I see it could not
> > break
> > > the installation.
> > > Nir
> > >
> > > On Tue, Nov 14, 2017 at 8:10 PM, Eric Friedrich (efriedri) <
> > > efrie...@cisco.com> wrote:
> > >
> > >> Thanks Matt-
> > >>  I found another LEGAL ticket (https://issues.apache.org/
> > >> jira/browse/LEGAL-330) based on a Google version of the PATENTS file
> > this
> > >> time.
> > >>
> > >> Looks like things are OK to use then.
> > >>
> > >> —Eric
> > >>
> > >> On Nov 14, 2017, at 12:50 PM, Mills, Matthew <
> matthew_mi...@comcast.com
> > <
> > >> mailto:matthew_mi...@comcast.com>> wrote:
> > >>
> > >> FYI, Go itself has the same file
> > >>
> > >> https://github.com/golang/go/blob/master/PATENTS
> > >>
> > >>
> > >> On 11/14/17, 10:36:43 AM, "Eric Friedrich (efriedri)" <
> > efrie...@cisco.com>
> > >> wrote:
> > >>
> > >>   I’ve been going through licensing for the 2.1 release and found this
> > >> file:
> > >>   ./traffic_stats/vendor/golang.org/x/net/PATENTS > >> golang.org/x/net/PATENTS>
> > >>
> > >>   This looks like it places some of the same restrictions that caused
> > the
> > >> whole Facebook React.js and rocksDb controversy a few months ago.
> > >>   Fun reading here: https://issues.apache.org/jira/browse/LEGAL-303
> > >>   There’s some in depth discussion of the detailed Facebook license, I
> > >> can’t even begin to speculate how that compares to this Google
> > conditional
> > >> patent grant.
> > >>
> > >>   We can see what the IPMC/Legal thinks or maybe just remove the code?
> > >>
> > >>   —Eric
> > >>
> > >>
> > >>
> > >>
> > >>
> >
> >
>