Re: Golang Proxy Validation Dependencies
+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 > Create/Updates. > Our current task is to circle back and start implementing the > (C)reate, > (U)pdate, and (D)eletes. One of the
Re: Golang Proxy Validation Dependencies
+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 like to leverage and provide > feedback: > > Both are MIT Licenses > Uses normal programming constructs rathe