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 <[email protected]> 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 <[email protected]> > 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 <[email protected]> > 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 <[email protected]> > 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 < > [email protected]> > >>> 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 > >>> >
