Re: Golang Proxy Validation Dependencies

[Volz, Dylan]

+1

On 1/17/18, 5:43 AM, "John Rushford"  wrote:

+1

Sent from my iPad

> On Jan 16, 2018, at 2:07 PM, Dave Neuman  wrote:
> 
> +1
> 
>> On Tue, Jan 16, 2018 at 12:58 Jan van Doorn  wrote:
>> 
>> +1 on using libs.
>> 
>>> On Jan 16, 2018, at 10:52 AM, Dan Kirkwood  wrote:
>>> 
>>> +1 -- agree with Jeff -- the validation of the fields of
>>> deliveryservice is something that is incomplete in the Perl
>>> traffic_ops.
>>> 
>>> These libraries make for concise code to do the validation so it will
>>> be easier to extend without much extra code.   It will not be called
>>> on every API function,  but only once on each POST/PUT which are a
>>> small minority of calls.   It also need not be used in every case.
>>> That, to me,  makes the reflection argument much less of a concern.
>>> 
>>> I would like to see it go in sooner than Friday,  but won't argue that
>> point..
>>> 
>>> -dan
>>> 
>>> 
>>> On Tue, Jan 16, 2018 at 10:10 AM, Dewayne Richardson 
>> wrote:
 So, it's been a few days on this topic and I'd like to call a vote for
>> the
 dependencies listed in this thread.  Please vote +1 or -1 by Noon
>> Friday so
 that we can move forward the Golang Proxy development.
 
 Thanks,
 
 -Dew
 
 On Thu, Jan 11, 2018 at 10:53 AM, Jeff Elsloo 
>> wrote:
 
> I don't think we should assume anything about the performance just
> because it uses reflection. Yes, traditionally reflection is
> computationally expensive, however, when used properly the penalty can
> be negligible. I don't think we have enough understanding of these
> libraries to know whether there is a concerning performance penalty.
> 
> As Dewayne said, create, update and delete actions represent a tiny
> fraction of the overall requests into TO. Given that the majority of
> these actions are performed by humans, I would be shocked if there was
> a perceptible performance difference with the reflection based
> validation in place. It's not like we're trying to validate enormous
> and complex objects here; we're talking 20 fields or so for any given
> post.
> 
> I'm +1 on using validation libraries such as these even if they use
> reflection, provided that we do not see dramatic changes in
> performance. I think that's highly unlikely in this case.
> --
> Thanks,
> Jeff
> 
> 
> On Thu, Jan 11, 2018 at 10:07 AM, Chris Lemmons 
> wrote:
>> True, but how many of those out-of-the-box checks are both useful and
>> relevantly complex?
>> 
>> To me, the cool part of ozzo is the way it collects the output and
>> formats it. That's unfortunately also the computationally expensive
>> part.
>> 
>> On Thu, Jan 11, 2018 at 9:49 AM, Dewayne Richardson <
>> dewr...@gmail.com>
> wrote:
>>> Please keep in mind that we do not Create/Update/Delete very often 
in
>>> Traffic Ops, so the performance penalty for Validation should be
>> taken
> into
>>> consideration.  I also don't want to re-invent all of those
> out-of-the-box
>>> field level checks by hand when I can just use them from here:
>>> https://github.com/asaskevich/govalidator#list-of-functions
>>> 
>>> -Dew
>>> 
>>> On Thu, Jan 11, 2018 at 9:24 AM, Chris Lemmons 
> wrote:
>>> 
 I like the output style, but I'm a bit concerned on the performance
 front. ozzo appears to do all it's magic with heavy use of
>> reflection,
 which is often a slow spot in go. Most places, it wouldn't matter
 much, but this will be called on every element of every API
>> function,
 so a nod toward performance may be in order. Have we done some
 measurement to see whether this adds a relevant amount of overhead
>> to
 the calls? Or are the calls still dominated by the DB lookup?
 
 Relatedly, is this a major advantage over something like this:
 
 if ds.Active == nil { errMsgs = append(errMsgs, `"active" must be
 provided`) }
 
 On Thu, Jan 11, 2018 at 8:49 AM, Dewayne Richardson <
> dewr...@apache.org>
 wrote:
> We've been moving along with more functionality in the Golang
>> proxy,
 mostly
> the Read's up until now, comparatively TO does much fewer
> 

Re: Golang Proxy Validation Dependencies

[John Rushford]

+1

Sent from my iPad

> On Jan 16, 2018, at 2:07 PM, Dave Neuman  wrote:
> 
> +1
> 
>> On Tue, Jan 16, 2018 at 12:58 Jan van Doorn  wrote:
>> 
>> +1 on using libs.
>> 
>>> On Jan 16, 2018, at 10:52 AM, Dan Kirkwood  wrote:
>>> 
>>> +1 -- agree with Jeff -- the validation of the fields of
>>> deliveryservice is something that is incomplete in the Perl
>>> traffic_ops.
>>> 
>>> These libraries make for concise code to do the validation so it will
>>> be easier to extend without much extra code.   It will not be called
>>> on every API function,  but only once on each POST/PUT which are a
>>> small minority of calls.   It also need not be used in every case.
>>> That, to me,  makes the reflection argument much less of a concern.
>>> 
>>> I would like to see it go in sooner than Friday,  but won't argue that
>> point..
>>> 
>>> -dan
>>> 
>>> 
>>> On Tue, Jan 16, 2018 at 10:10 AM, Dewayne Richardson 
>> wrote:
 So, it's been a few days on this topic and I'd like to call a vote for
>> the
 dependencies listed in this thread.  Please vote +1 or -1 by Noon
>> Friday so
 that we can move forward the Golang Proxy development.
 
 Thanks,
 
 -Dew
 
 On Thu, Jan 11, 2018 at 10:53 AM, Jeff Elsloo 
>> wrote:
 
> I don't think we should assume anything about the performance just
> because it uses reflection. Yes, traditionally reflection is
> computationally expensive, however, when used properly the penalty can
> be negligible. I don't think we have enough understanding of these
> libraries to know whether there is a concerning performance penalty.
> 
> As Dewayne said, create, update and delete actions represent a tiny
> fraction of the overall requests into TO. Given that the majority of
> these actions are performed by humans, I would be shocked if there was
> a perceptible performance difference with the reflection based
> validation in place. It's not like we're trying to validate enormous
> and complex objects here; we're talking 20 fields or so for any given
> post.
> 
> I'm +1 on using validation libraries such as these even if they use
> reflection, provided that we do not see dramatic changes in
> performance. I think that's highly unlikely in this case.
> --
> Thanks,
> Jeff
> 
> 
> On Thu, Jan 11, 2018 at 10:07 AM, Chris Lemmons 
> wrote:
>> True, but how many of those out-of-the-box checks are both useful and
>> relevantly complex?
>> 
>> To me, the cool part of ozzo is the way it collects the output and
>> formats it. That's unfortunately also the computationally expensive
>> part.
>> 
>> On Thu, Jan 11, 2018 at 9:49 AM, Dewayne Richardson <
>> dewr...@gmail.com>
> wrote:
>>> Please keep in mind that we do not Create/Update/Delete very often in
>>> Traffic Ops, so the performance penalty for Validation should be
>> taken
> into
>>> consideration.  I also don't want to re-invent all of those
> out-of-the-box
>>> field level checks by hand when I can just use them from here:
>>> https://github.com/asaskevich/govalidator#list-of-functions
>>> 
>>> -Dew
>>> 
>>> On Thu, Jan 11, 2018 at 9:24 AM, Chris Lemmons 
> wrote:
>>> 
 I like the output style, but I'm a bit concerned on the performance
 front. ozzo appears to do all it's magic with heavy use of
>> reflection,
 which is often a slow spot in go. Most places, it wouldn't matter
 much, but this will be called on every element of every API
>> function,
 so a nod toward performance may be in order. Have we done some
 measurement to see whether this adds a relevant amount of overhead
>> to
 the calls? Or are the calls still dominated by the DB lookup?
 
 Relatedly, is this a major advantage over something like this:
 
 if ds.Active == nil { errMsgs = append(errMsgs, `"active" must be
 provided`) }
 
 On Thu, Jan 11, 2018 at 8:49 AM, Dewayne Richardson <
> dewr...@apache.org>
 wrote:
> We've been moving along with more functionality in the Golang
>> proxy,
 mostly
> the Read's up until now, comparatively TO does much fewer
> Create/Updates.
> Our current task is to circle back and start implementing the
> (C)reate,
> (U)pdate, and (D)eletes.  One of the obvious needs for the this
>> task
> are
> validation rules.  I've been doing research to figure out the
> cleanest
 and
> most maintainable way to rewrite the Perl validation rules in Go.
> 
> TC Issue for tracking
> https://github.com/apache/incubator-trafficcontrol/issues/1756
> 
> These are the two dependencies I'd