You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficcontrol.apache.org by Dewayne Richardson <de...@apache.org> on 2018/01/11 15:49:33 UTC
Golang Proxy Validation Dependencies
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 like to leverage and provide feedback:
Both are MIT Licenses
Uses normal programming constructs rather than error-prone struct tags to
specify how data should be validated.
https://github.com/go-ozzo/ozzo-validation
https://github.com/go-ozzo/ozzo-validation/blob/master/LICENSE
Core Validation library that the prior library uses that has a lot of
useful convenience methods that I'd rather not re-invent
https://github.com/asaskevich/govalidator
https://github.com/asaskevich/govalidator#list-of-functions
https://github.com/asaskevich/govalidator/blob/master/LICENSE
And here is how I've used these as sample validation rules that I've
implemented as a POC:
https://github.com/dewrich/incubator-trafficcontrol/blob/tor-api-ds/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go#L93
Existing Mojo Perl Rules for comparison.
https://github.com/apache/incubator-trafficcontrol/blob/master/traffic_ops/app/lib/API/Deliveryservice.pm#L1363
-Dew
Re: Golang Proxy Validation Dependencies
Posted by Dewayne Richardson <de...@gmail.com>.
So, for the vote of using the validation dependencies we had
5 +1's for using the Validation libs
So the vote passes, that these dependencies will be used for the Golang
Proxy validation rules.
This PR will be can now be merged:
https://github.com/apache/incubator-trafficcontrol/pull/1766
Thank you for taking the time to respond,
-Dew
On Wed, Jan 17, 2018 at 9:25 AM, Volz, Dylan <Dy...@comcast.com> wrote:
> +1
>
> On 1/17/18, 5:43 AM, "John Rushford" <jj...@gmail.com> wrote:
>
> +1
>
> Sent from my iPad
>
> > On Jan 16, 2018, at 2:07 PM, Dave Neuman <ne...@apache.org> wrote:
> >
> > +1
> >
> >> On Tue, Jan 16, 2018 at 12:58 Jan van Doorn <jv...@knutsel.com>
> wrote:
> >>
> >> +1 on using libs.
> >>
> >>> On Jan 16, 2018, at 10:52 AM, Dan Kirkwood <da...@gmail.com>
> 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 <
> dewrich@gmail.com>
> >> 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 <el...@apache.org>
> >> 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 <
> alficles@gmail.com>
> >>>>> 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 <
> >> dewrich@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 <
> alficles@gmail.com>
> >>>>> 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 <
> >>>>> dewrich@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 like to leverage and
> provide
> >>>>> feedback:
> >>>>>>>>>
> >>>>>>>>> Both are MIT Licenses
> >>>>>>>>> Uses normal programming constructs rather than error-prone
> struct
> >>>>> tags to
> >>>>>>>>> specify how data should be validated.
> >>>>>>>>> https://github.com/go-ozzo/ozzo-validation
> >>>>>>>>> https://github.com/go-ozzo/ozzo-validation/blob/master/
> LICENSE
> >>>>>>>>>
> >>>>>>>>> Core Validation library that the prior library uses that has
> a lot
> >> of
> >>>>>>>>> useful convenience methods that I'd rather not re-invent
> >>>>>>>>> https://github.com/asaskevich/govalidator
> >>>>>>>>> https://github.com/asaskevich/govalidator#list-of-functions
> >>>>>>>>> https://github.com/asaskevich/govalidator/blob/master/
> LICENSE
> >>>>>>>>>
> >>>>>>>>> And here is how I've used these as sample validation rules
> that
> >> I've
> >>>>>>>>> implemented as a POC:
> >>>>>>>>>
> >>>>>>>>> https://github.com/dewrich/incubator-trafficcontrol/blob/
> >>>>>>>> tor-api-ds/traffic_ops/traffic_ops_golang/deliveryservice/
> >>>>>>>> deliveryservices.go#L93
> >>>>>>>>>
> >>>>>>>>> Existing Mojo Perl Rules for comparison.
> >>>>>>>>> https://github.com/apache/incubator-trafficcontrol/blob/
> >>>>>>>> master/traffic_ops/app/lib/API/Deliveryservice.pm#L1363
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> -Dew
> >>>>>>>>
> >>>>>
> >>
> >>
>
>
>
>
Re: Golang Proxy Validation Dependencies
Posted by "Volz, Dylan" <Dy...@comcast.com>.
+1
On 1/17/18, 5:43 AM, "John Rushford" <jj...@gmail.com> wrote:
+1
Sent from my iPad
> On Jan 16, 2018, at 2:07 PM, Dave Neuman <ne...@apache.org> wrote:
>
> +1
>
>> On Tue, Jan 16, 2018 at 12:58 Jan van Doorn <jv...@knutsel.com> wrote:
>>
>> +1 on using libs.
>>
>>> On Jan 16, 2018, at 10:52 AM, Dan Kirkwood <da...@gmail.com> 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 <de...@gmail.com>
>> 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 <el...@apache.org>
>> 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 <al...@gmail.com>
>>>>> 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 <
>> dewrich@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 <al...@gmail.com>
>>>>> 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 <
>>>>> dewrich@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 like to leverage and provide
>>>>> feedback:
>>>>>>>>>
>>>>>>>>> Both are MIT Licenses
>>>>>>>>> Uses normal programming constructs rather than error-prone struct
>>>>> tags to
>>>>>>>>> specify how data should be validated.
>>>>>>>>> https://github.com/go-ozzo/ozzo-validation
>>>>>>>>> https://github.com/go-ozzo/ozzo-validation/blob/master/LICENSE
>>>>>>>>>
>>>>>>>>> Core Validation library that the prior library uses that has a lot
>> of
>>>>>>>>> useful convenience methods that I'd rather not re-invent
>>>>>>>>> https://github.com/asaskevich/govalidator
>>>>>>>>> https://github.com/asaskevich/govalidator#list-of-functions
>>>>>>>>> https://github.com/asaskevich/govalidator/blob/master/LICENSE
>>>>>>>>>
>>>>>>>>> And here is how I've used these as sample validation rules that
>> I've
>>>>>>>>> implemented as a POC:
>>>>>>>>>
>>>>>>>>> https://github.com/dewrich/incubator-trafficcontrol/blob/
>>>>>>>> tor-api-ds/traffic_ops/traffic_ops_golang/deliveryservice/
>>>>>>>> deliveryservices.go#L93
>>>>>>>>>
>>>>>>>>> Existing Mojo Perl Rules for comparison.
>>>>>>>>> https://github.com/apache/incubator-trafficcontrol/blob/
>>>>>>>> master/traffic_ops/app/lib/API/Deliveryservice.pm#L1363
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> -Dew
>>>>>>>>
>>>>>
>>
>>
Re: Golang Proxy Validation Dependencies
Posted by John Rushford <jj...@gmail.com>.
+1
Sent from my iPad
> On Jan 16, 2018, at 2:07 PM, Dave Neuman <ne...@apache.org> wrote:
>
> +1
>
>> On Tue, Jan 16, 2018 at 12:58 Jan van Doorn <jv...@knutsel.com> wrote:
>>
>> +1 on using libs.
>>
>>> On Jan 16, 2018, at 10:52 AM, Dan Kirkwood <da...@gmail.com> 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 <de...@gmail.com>
>> 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 <el...@apache.org>
>> 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 <al...@gmail.com>
>>>>> 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 <
>> dewrich@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 <al...@gmail.com>
>>>>> 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 <
>>>>> dewrich@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 like to leverage and provide
>>>>> feedback:
>>>>>>>>>
>>>>>>>>> Both are MIT Licenses
>>>>>>>>> Uses normal programming constructs rather than error-prone struct
>>>>> tags to
>>>>>>>>> specify how data should be validated.
>>>>>>>>> https://github.com/go-ozzo/ozzo-validation
>>>>>>>>> https://github.com/go-ozzo/ozzo-validation/blob/master/LICENSE
>>>>>>>>>
>>>>>>>>> Core Validation library that the prior library uses that has a lot
>> of
>>>>>>>>> useful convenience methods that I'd rather not re-invent
>>>>>>>>> https://github.com/asaskevich/govalidator
>>>>>>>>> https://github.com/asaskevich/govalidator#list-of-functions
>>>>>>>>> https://github.com/asaskevich/govalidator/blob/master/LICENSE
>>>>>>>>>
>>>>>>>>> And here is how I've used these as sample validation rules that
>> I've
>>>>>>>>> implemented as a POC:
>>>>>>>>>
>>>>>>>>> https://github.com/dewrich/incubator-trafficcontrol/blob/
>>>>>>>> tor-api-ds/traffic_ops/traffic_ops_golang/deliveryservice/
>>>>>>>> deliveryservices.go#L93
>>>>>>>>>
>>>>>>>>> Existing Mojo Perl Rules for comparison.
>>>>>>>>> https://github.com/apache/incubator-trafficcontrol/blob/
>>>>>>>> master/traffic_ops/app/lib/API/Deliveryservice.pm#L1363
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> -Dew
>>>>>>>>
>>>>>
>>
>>
Re: Golang Proxy Validation Dependencies
Posted by Dave Neuman <ne...@apache.org>.
+1
On Tue, Jan 16, 2018 at 12:58 Jan van Doorn <jv...@knutsel.com> wrote:
> +1 on using libs.
>
> > On Jan 16, 2018, at 10:52 AM, Dan Kirkwood <da...@gmail.com> 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 <de...@gmail.com>
> 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 <el...@apache.org>
> 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 <al...@gmail.com>
> >>> 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 <
> dewrich@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 <al...@gmail.com>
> >>> 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 <
> >>> dewrich@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 like to leverage and provide
> >>> feedback:
> >>>>>>>
> >>>>>>> Both are MIT Licenses
> >>>>>>> Uses normal programming constructs rather than error-prone struct
> >>> tags to
> >>>>>>> specify how data should be validated.
> >>>>>>> https://github.com/go-ozzo/ozzo-validation
> >>>>>>> https://github.com/go-ozzo/ozzo-validation/blob/master/LICENSE
> >>>>>>>
> >>>>>>> Core Validation library that the prior library uses that has a lot
> of
> >>>>>>> useful convenience methods that I'd rather not re-invent
> >>>>>>> https://github.com/asaskevich/govalidator
> >>>>>>> https://github.com/asaskevich/govalidator#list-of-functions
> >>>>>>> https://github.com/asaskevich/govalidator/blob/master/LICENSE
> >>>>>>>
> >>>>>>> And here is how I've used these as sample validation rules that
> I've
> >>>>>>> implemented as a POC:
> >>>>>>>
> >>>>>>> https://github.com/dewrich/incubator-trafficcontrol/blob/
> >>>>>> tor-api-ds/traffic_ops/traffic_ops_golang/deliveryservice/
> >>>>>> deliveryservices.go#L93
> >>>>>>>
> >>>>>>> Existing Mojo Perl Rules for comparison.
> >>>>>>> https://github.com/apache/incubator-trafficcontrol/blob/
> >>>>>> master/traffic_ops/app/lib/API/Deliveryservice.pm#L1363
> >>>>>>>
> >>>>>>>
> >>>>>>> -Dew
> >>>>>>
> >>>
>
>
Re: Golang Proxy Validation Dependencies
Posted by Jan van Doorn <jv...@knutsel.com>.
+1 on using libs.
> On Jan 16, 2018, at 10:52 AM, Dan Kirkwood <da...@gmail.com> 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 <de...@gmail.com> 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 <el...@apache.org> 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 <al...@gmail.com>
>>> 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 <de...@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 <al...@gmail.com>
>>> 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 <
>>> dewrich@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 like to leverage and provide
>>> feedback:
>>>>>>>
>>>>>>> Both are MIT Licenses
>>>>>>> Uses normal programming constructs rather than error-prone struct
>>> tags to
>>>>>>> specify how data should be validated.
>>>>>>> https://github.com/go-ozzo/ozzo-validation
>>>>>>> https://github.com/go-ozzo/ozzo-validation/blob/master/LICENSE
>>>>>>>
>>>>>>> Core Validation library that the prior library uses that has a lot of
>>>>>>> useful convenience methods that I'd rather not re-invent
>>>>>>> https://github.com/asaskevich/govalidator
>>>>>>> https://github.com/asaskevich/govalidator#list-of-functions
>>>>>>> https://github.com/asaskevich/govalidator/blob/master/LICENSE
>>>>>>>
>>>>>>> And here is how I've used these as sample validation rules that I've
>>>>>>> implemented as a POC:
>>>>>>>
>>>>>>> https://github.com/dewrich/incubator-trafficcontrol/blob/
>>>>>> tor-api-ds/traffic_ops/traffic_ops_golang/deliveryservice/
>>>>>> deliveryservices.go#L93
>>>>>>>
>>>>>>> Existing Mojo Perl Rules for comparison.
>>>>>>> https://github.com/apache/incubator-trafficcontrol/blob/
>>>>>> master/traffic_ops/app/lib/API/Deliveryservice.pm#L1363
>>>>>>>
>>>>>>>
>>>>>>> -Dew
>>>>>>
>>>
Re: Golang Proxy Validation Dependencies
Posted by Dan Kirkwood <da...@gmail.com>.
+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 <de...@gmail.com> 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 <el...@apache.org> 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 <al...@gmail.com>
>> 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 <de...@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 <al...@gmail.com>
>> 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 <
>> dewrich@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 like to leverage and provide
>> feedback:
>> >>> >
>> >>> > Both are MIT Licenses
>> >>> > Uses normal programming constructs rather than error-prone struct
>> tags to
>> >>> > specify how data should be validated.
>> >>> > https://github.com/go-ozzo/ozzo-validation
>> >>> > https://github.com/go-ozzo/ozzo-validation/blob/master/LICENSE
>> >>> >
>> >>> > Core Validation library that the prior library uses that has a lot of
>> >>> > useful convenience methods that I'd rather not re-invent
>> >>> > https://github.com/asaskevich/govalidator
>> >>> > https://github.com/asaskevich/govalidator#list-of-functions
>> >>> > https://github.com/asaskevich/govalidator/blob/master/LICENSE
>> >>> >
>> >>> > And here is how I've used these as sample validation rules that I've
>> >>> > implemented as a POC:
>> >>> >
>> >>> > https://github.com/dewrich/incubator-trafficcontrol/blob/
>> >>> tor-api-ds/traffic_ops/traffic_ops_golang/deliveryservice/
>> >>> deliveryservices.go#L93
>> >>> >
>> >>> > Existing Mojo Perl Rules for comparison.
>> >>> > https://github.com/apache/incubator-trafficcontrol/blob/
>> >>> master/traffic_ops/app/lib/API/Deliveryservice.pm#L1363
>> >>> >
>> >>> >
>> >>> > -Dew
>> >>>
>>
Re: Golang Proxy Validation Dependencies
Posted by Dewayne Richardson <de...@gmail.com>.
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 <el...@apache.org> 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 <al...@gmail.com>
> 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 <de...@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 <al...@gmail.com>
> 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 <
> dewrich@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 like to leverage and provide
> feedback:
> >>> >
> >>> > Both are MIT Licenses
> >>> > Uses normal programming constructs rather than error-prone struct
> tags to
> >>> > specify how data should be validated.
> >>> > https://github.com/go-ozzo/ozzo-validation
> >>> > https://github.com/go-ozzo/ozzo-validation/blob/master/LICENSE
> >>> >
> >>> > Core Validation library that the prior library uses that has a lot of
> >>> > useful convenience methods that I'd rather not re-invent
> >>> > https://github.com/asaskevich/govalidator
> >>> > https://github.com/asaskevich/govalidator#list-of-functions
> >>> > https://github.com/asaskevich/govalidator/blob/master/LICENSE
> >>> >
> >>> > And here is how I've used these as sample validation rules that I've
> >>> > implemented as a POC:
> >>> >
> >>> > https://github.com/dewrich/incubator-trafficcontrol/blob/
> >>> tor-api-ds/traffic_ops/traffic_ops_golang/deliveryservice/
> >>> deliveryservices.go#L93
> >>> >
> >>> > Existing Mojo Perl Rules for comparison.
> >>> > https://github.com/apache/incubator-trafficcontrol/blob/
> >>> master/traffic_ops/app/lib/API/Deliveryservice.pm#L1363
> >>> >
> >>> >
> >>> > -Dew
> >>>
>
Re: Golang Proxy Validation Dependencies
Posted by Jeff Elsloo <el...@apache.org>.
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 <al...@gmail.com> 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 <de...@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 <al...@gmail.com> 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 <de...@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 like to leverage and provide feedback:
>>> >
>>> > Both are MIT Licenses
>>> > Uses normal programming constructs rather than error-prone struct tags to
>>> > specify how data should be validated.
>>> > https://github.com/go-ozzo/ozzo-validation
>>> > https://github.com/go-ozzo/ozzo-validation/blob/master/LICENSE
>>> >
>>> > Core Validation library that the prior library uses that has a lot of
>>> > useful convenience methods that I'd rather not re-invent
>>> > https://github.com/asaskevich/govalidator
>>> > https://github.com/asaskevich/govalidator#list-of-functions
>>> > https://github.com/asaskevich/govalidator/blob/master/LICENSE
>>> >
>>> > And here is how I've used these as sample validation rules that I've
>>> > implemented as a POC:
>>> >
>>> > https://github.com/dewrich/incubator-trafficcontrol/blob/
>>> tor-api-ds/traffic_ops/traffic_ops_golang/deliveryservice/
>>> deliveryservices.go#L93
>>> >
>>> > Existing Mojo Perl Rules for comparison.
>>> > https://github.com/apache/incubator-trafficcontrol/blob/
>>> master/traffic_ops/app/lib/API/Deliveryservice.pm#L1363
>>> >
>>> >
>>> > -Dew
>>>
Re: Golang Proxy Validation Dependencies
Posted by Chris Lemmons <al...@gmail.com>.
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 <de...@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 <al...@gmail.com> 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 <de...@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 like to leverage and provide feedback:
>> >
>> > Both are MIT Licenses
>> > Uses normal programming constructs rather than error-prone struct tags to
>> > specify how data should be validated.
>> > https://github.com/go-ozzo/ozzo-validation
>> > https://github.com/go-ozzo/ozzo-validation/blob/master/LICENSE
>> >
>> > Core Validation library that the prior library uses that has a lot of
>> > useful convenience methods that I'd rather not re-invent
>> > https://github.com/asaskevich/govalidator
>> > https://github.com/asaskevich/govalidator#list-of-functions
>> > https://github.com/asaskevich/govalidator/blob/master/LICENSE
>> >
>> > And here is how I've used these as sample validation rules that I've
>> > implemented as a POC:
>> >
>> > https://github.com/dewrich/incubator-trafficcontrol/blob/
>> tor-api-ds/traffic_ops/traffic_ops_golang/deliveryservice/
>> deliveryservices.go#L93
>> >
>> > Existing Mojo Perl Rules for comparison.
>> > https://github.com/apache/incubator-trafficcontrol/blob/
>> master/traffic_ops/app/lib/API/Deliveryservice.pm#L1363
>> >
>> >
>> > -Dew
>>
Re: Golang Proxy Validation Dependencies
Posted by Dewayne Richardson <de...@gmail.com>.
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 <al...@gmail.com> 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 <de...@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 like to leverage and provide feedback:
> >
> > Both are MIT Licenses
> > Uses normal programming constructs rather than error-prone struct tags to
> > specify how data should be validated.
> > https://github.com/go-ozzo/ozzo-validation
> > https://github.com/go-ozzo/ozzo-validation/blob/master/LICENSE
> >
> > Core Validation library that the prior library uses that has a lot of
> > useful convenience methods that I'd rather not re-invent
> > https://github.com/asaskevich/govalidator
> > https://github.com/asaskevich/govalidator#list-of-functions
> > https://github.com/asaskevich/govalidator/blob/master/LICENSE
> >
> > And here is how I've used these as sample validation rules that I've
> > implemented as a POC:
> >
> > https://github.com/dewrich/incubator-trafficcontrol/blob/
> tor-api-ds/traffic_ops/traffic_ops_golang/deliveryservice/
> deliveryservices.go#L93
> >
> > Existing Mojo Perl Rules for comparison.
> > https://github.com/apache/incubator-trafficcontrol/blob/
> master/traffic_ops/app/lib/API/Deliveryservice.pm#L1363
> >
> >
> > -Dew
>
Re: Golang Proxy Validation Dependencies
Posted by Chris Lemmons <al...@gmail.com>.
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 <de...@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 like to leverage and provide feedback:
>
> Both are MIT Licenses
> Uses normal programming constructs rather than error-prone struct tags to
> specify how data should be validated.
> https://github.com/go-ozzo/ozzo-validation
> https://github.com/go-ozzo/ozzo-validation/blob/master/LICENSE
>
> Core Validation library that the prior library uses that has a lot of
> useful convenience methods that I'd rather not re-invent
> https://github.com/asaskevich/govalidator
> https://github.com/asaskevich/govalidator#list-of-functions
> https://github.com/asaskevich/govalidator/blob/master/LICENSE
>
> And here is how I've used these as sample validation rules that I've
> implemented as a POC:
>
> https://github.com/dewrich/incubator-trafficcontrol/blob/tor-api-ds/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go#L93
>
> Existing Mojo Perl Rules for comparison.
> https://github.com/apache/incubator-trafficcontrol/blob/master/traffic_ops/app/lib/API/Deliveryservice.pm#L1363
>
>
> -Dew