+1 on using libs.
> On Jan 16, 2018, at 10:52 AM, Dan Kirkwood <[email protected]> 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 <[email protected]>
> 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 <[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
>>>>>>
>>>
