You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficcontrol.apache.org by Rawlin Peters <ra...@gmail.com> on 2018/04/25 21:53:14 UTC

Traffic Ops go client responses

Hey folks,

I noticed in our TO go client that we aren't decoding the JSON
response returned from PUT/POST endpoints. If we actually decoded
those responses, it would be quicker and more useful from a user's
perspective, save bandwidth from all the unnecessary GETs after
POST/PUTs, and also reduce load on the API server.

Is there a reason why we don't decode those responses?

- Rawlin

Re: Traffic Ops go client responses

Posted by Dewayne Richardson <de...@apache.org>.
I think at one point there was some effort to expose some of that with the
"rawRequest" func, not sure who created a lot of these, I have just been
using what was there for the API Tests.  I think we do need to do some
refactoring to expose more for proper API Testing.

https://github.com/apache/incubator-trafficcontrol/blob/master/traffic_ops/client/v13/session.go#L285

-Dew

On Wed, Apr 25, 2018 at 3:53 PM, Rawlin Peters <ra...@gmail.com>
wrote:

> Hey folks,
>
> I noticed in our TO go client that we aren't decoding the JSON
> response returned from PUT/POST endpoints. If we actually decoded
> those responses, it would be quicker and more useful from a user's
> perspective, save bandwidth from all the unnecessary GETs after
> POST/PUTs, and also reduce load on the API server.
>
> Is there a reason why we don't decode those responses?
>
> - Rawlin
>

Re: Traffic Ops go client responses

Posted by Robert Butts <ro...@gmail.com>.
>most if not all of the files in the client/v13 directory came after
2.2.0-RC1, and assuming that's true, do we really have an obligation to
follow the deprecation process for unreleased code?

Right, if no function with the same signature exists in Release 2.1 or
older, we can change them right now.

Though, even if it isn't released, we may still have code in master using
them. We'll need to check everything in master, and fix any usages.

We'll need to especially double-check any usage of the Get functions, which
return the object, which will still compile with Go's type inference, and
nil panic at runtime.



On Thu, Apr 26, 2018 at 3:06 PM, Rawlin Peters <ra...@gmail.com>
wrote:

> Ok, I wasn't sure if this was done intentionally as part of some
> framework planning or not. It would seem that most if not all of the
> files in the client/v13 directory came after 2.2.0-RC1, and assuming
> that's true, do we really have an obligation to follow the deprecation
> process for unreleased code?
>
> I might be wrong about the age of some of those files, but I know at
> least a handful of them were added after 2.2.0-RC1 for sure.
>
> - Rawlin
>
> On Thu, Apr 26, 2018 at 12:27 PM, Robert Butts <ro...@gmail.com>
> wrote:
> >>client methods take the Foo struct as input rather than the FooNullable
> > struct, so it's impossible to create a request with null fields
> >
> > It's a legacy artifact. You're right, it's a serious limitation, it's
> just
> > never been a priority to fix.
> >
> > The client is a public interface to the API, we can't just change it and
> > break people. I propose we add new functions, e.g.
> `CreateTenantNullable`,
> > and mark the existing functions as deprecated, and remove them in the
> next
> > major version.
> >
> >
> > On Thu, Apr 26, 2018 at 12:02 PM, Rawlin Peters <rawlin.peters@gmail.com
> >
> > wrote:
> >
> >> Digging a little further into this, it looks like in the older client
> >> we actually did decode the json response from PUT/POST requests and
> >> return it, as evidenced by this CreateTenant method signature:
> >>
> >> func (to *Session) CreateTenant(t *tc.Tenant) (*tc.TenantResponse,
> error) {
> >>
> >> Now I'm really confused as to why we've deviated from this behavior in
> >> the 1.3 client so far.
> >>
> >> Another issue is that the client methods take the Foo struct as input
> >> rather than the FooNullable struct, so it's impossible to create a
> >> request with null fields where applicable. Does anyone know of a good
> >> reason why we're implementing the new client like this?
> >>
> >> - Rawlin
> >>
> >> On Thu, Apr 26, 2018 at 11:11 AM, Rawlin Peters <
> rawlin.peters@gmail.com>
> >> wrote:
> >> > If you've found some Perl endpoints that aren't returning the modified
> >> > record, I think that's definitely a bug, and there should be GitHub
> >> > issues filed to fix them (unless they're already fixed in the Go
> >> > implementation). The API should always return the modified resource,
> >> > and the client should have immediate access to the modified resource
> >> > without needing a subsequent GET after every PUT/POST.
> >> >
> >> > At least for some new Go endpoints I'm working on, I'm going to make
> >> > the client decode the json responses, but I think we should definitely
> >> > consider making that the standard with our API clients.
> >> >
> >> > -Rawlin
> >> >
> >> > On Wed, Apr 25, 2018 at 7:14 PM, Schmidt, Andrew (Contractor)
> >> > <An...@comcast.com> wrote:
> >> >> I've noticed that the Put/Post responses are incorrect from some of
> the
> >> Perl endpoints. I'm guessing maybe that is the reason. For example the
> Put
> >> response returns the unmodified record. You have to do a Get to verify
> that
> >> the change occurred.
> >> >>
> >> >> Andy
> >> >>
> >> >> On 4/25/18, 3:53 PM, "Rawlin Peters" <ra...@gmail.com>
> wrote:
> >> >>
> >> >>     Hey folks,
> >> >>
> >> >>     I noticed in our TO go client that we aren't decoding the JSON
> >> >>     response returned from PUT/POST endpoints. If we actually decoded
> >> >>     those responses, it would be quicker and more useful from a
> user's
> >> >>     perspective, save bandwidth from all the unnecessary GETs after
> >> >>     POST/PUTs, and also reduce load on the API server.
> >> >>
> >> >>     Is there a reason why we don't decode those responses?
> >> >>
> >> >>     - Rawlin
> >> >>
> >> >>
> >> >>
> >>
>

Re: Traffic Ops go client responses

Posted by Rawlin Peters <ra...@gmail.com>.
Ok, I wasn't sure if this was done intentionally as part of some
framework planning or not. It would seem that most if not all of the
files in the client/v13 directory came after 2.2.0-RC1, and assuming
that's true, do we really have an obligation to follow the deprecation
process for unreleased code?

I might be wrong about the age of some of those files, but I know at
least a handful of them were added after 2.2.0-RC1 for sure.

- Rawlin

On Thu, Apr 26, 2018 at 12:27 PM, Robert Butts <ro...@gmail.com> wrote:
>>client methods take the Foo struct as input rather than the FooNullable
> struct, so it's impossible to create a request with null fields
>
> It's a legacy artifact. You're right, it's a serious limitation, it's just
> never been a priority to fix.
>
> The client is a public interface to the API, we can't just change it and
> break people. I propose we add new functions, e.g. `CreateTenantNullable`,
> and mark the existing functions as deprecated, and remove them in the next
> major version.
>
>
> On Thu, Apr 26, 2018 at 12:02 PM, Rawlin Peters <ra...@gmail.com>
> wrote:
>
>> Digging a little further into this, it looks like in the older client
>> we actually did decode the json response from PUT/POST requests and
>> return it, as evidenced by this CreateTenant method signature:
>>
>> func (to *Session) CreateTenant(t *tc.Tenant) (*tc.TenantResponse, error) {
>>
>> Now I'm really confused as to why we've deviated from this behavior in
>> the 1.3 client so far.
>>
>> Another issue is that the client methods take the Foo struct as input
>> rather than the FooNullable struct, so it's impossible to create a
>> request with null fields where applicable. Does anyone know of a good
>> reason why we're implementing the new client like this?
>>
>> - Rawlin
>>
>> On Thu, Apr 26, 2018 at 11:11 AM, Rawlin Peters <ra...@gmail.com>
>> wrote:
>> > If you've found some Perl endpoints that aren't returning the modified
>> > record, I think that's definitely a bug, and there should be GitHub
>> > issues filed to fix them (unless they're already fixed in the Go
>> > implementation). The API should always return the modified resource,
>> > and the client should have immediate access to the modified resource
>> > without needing a subsequent GET after every PUT/POST.
>> >
>> > At least for some new Go endpoints I'm working on, I'm going to make
>> > the client decode the json responses, but I think we should definitely
>> > consider making that the standard with our API clients.
>> >
>> > -Rawlin
>> >
>> > On Wed, Apr 25, 2018 at 7:14 PM, Schmidt, Andrew (Contractor)
>> > <An...@comcast.com> wrote:
>> >> I've noticed that the Put/Post responses are incorrect from some of the
>> Perl endpoints. I'm guessing maybe that is the reason. For example the Put
>> response returns the unmodified record. You have to do a Get to verify that
>> the change occurred.
>> >>
>> >> Andy
>> >>
>> >> On 4/25/18, 3:53 PM, "Rawlin Peters" <ra...@gmail.com> wrote:
>> >>
>> >>     Hey folks,
>> >>
>> >>     I noticed in our TO go client that we aren't decoding the JSON
>> >>     response returned from PUT/POST endpoints. If we actually decoded
>> >>     those responses, it would be quicker and more useful from a user's
>> >>     perspective, save bandwidth from all the unnecessary GETs after
>> >>     POST/PUTs, and also reduce load on the API server.
>> >>
>> >>     Is there a reason why we don't decode those responses?
>> >>
>> >>     - Rawlin
>> >>
>> >>
>> >>
>>

Re: Traffic Ops go client responses

Posted by Robert Butts <ro...@gmail.com>.
>client methods take the Foo struct as input rather than the FooNullable
struct, so it's impossible to create a request with null fields

It's a legacy artifact. You're right, it's a serious limitation, it's just
never been a priority to fix.

The client is a public interface to the API, we can't just change it and
break people. I propose we add new functions, e.g. `CreateTenantNullable`,
and mark the existing functions as deprecated, and remove them in the next
major version.


On Thu, Apr 26, 2018 at 12:02 PM, Rawlin Peters <ra...@gmail.com>
wrote:

> Digging a little further into this, it looks like in the older client
> we actually did decode the json response from PUT/POST requests and
> return it, as evidenced by this CreateTenant method signature:
>
> func (to *Session) CreateTenant(t *tc.Tenant) (*tc.TenantResponse, error) {
>
> Now I'm really confused as to why we've deviated from this behavior in
> the 1.3 client so far.
>
> Another issue is that the client methods take the Foo struct as input
> rather than the FooNullable struct, so it's impossible to create a
> request with null fields where applicable. Does anyone know of a good
> reason why we're implementing the new client like this?
>
> - Rawlin
>
> On Thu, Apr 26, 2018 at 11:11 AM, Rawlin Peters <ra...@gmail.com>
> wrote:
> > If you've found some Perl endpoints that aren't returning the modified
> > record, I think that's definitely a bug, and there should be GitHub
> > issues filed to fix them (unless they're already fixed in the Go
> > implementation). The API should always return the modified resource,
> > and the client should have immediate access to the modified resource
> > without needing a subsequent GET after every PUT/POST.
> >
> > At least for some new Go endpoints I'm working on, I'm going to make
> > the client decode the json responses, but I think we should definitely
> > consider making that the standard with our API clients.
> >
> > -Rawlin
> >
> > On Wed, Apr 25, 2018 at 7:14 PM, Schmidt, Andrew (Contractor)
> > <An...@comcast.com> wrote:
> >> I've noticed that the Put/Post responses are incorrect from some of the
> Perl endpoints. I'm guessing maybe that is the reason. For example the Put
> response returns the unmodified record. You have to do a Get to verify that
> the change occurred.
> >>
> >> Andy
> >>
> >> On 4/25/18, 3:53 PM, "Rawlin Peters" <ra...@gmail.com> wrote:
> >>
> >>     Hey folks,
> >>
> >>     I noticed in our TO go client that we aren't decoding the JSON
> >>     response returned from PUT/POST endpoints. If we actually decoded
> >>     those responses, it would be quicker and more useful from a user's
> >>     perspective, save bandwidth from all the unnecessary GETs after
> >>     POST/PUTs, and also reduce load on the API server.
> >>
> >>     Is there a reason why we don't decode those responses?
> >>
> >>     - Rawlin
> >>
> >>
> >>
>

Re: Traffic Ops go client responses

Posted by Rawlin Peters <ra...@gmail.com>.
Digging a little further into this, it looks like in the older client
we actually did decode the json response from PUT/POST requests and
return it, as evidenced by this CreateTenant method signature:

func (to *Session) CreateTenant(t *tc.Tenant) (*tc.TenantResponse, error) {

Now I'm really confused as to why we've deviated from this behavior in
the 1.3 client so far.

Another issue is that the client methods take the Foo struct as input
rather than the FooNullable struct, so it's impossible to create a
request with null fields where applicable. Does anyone know of a good
reason why we're implementing the new client like this?

- Rawlin

On Thu, Apr 26, 2018 at 11:11 AM, Rawlin Peters <ra...@gmail.com> wrote:
> If you've found some Perl endpoints that aren't returning the modified
> record, I think that's definitely a bug, and there should be GitHub
> issues filed to fix them (unless they're already fixed in the Go
> implementation). The API should always return the modified resource,
> and the client should have immediate access to the modified resource
> without needing a subsequent GET after every PUT/POST.
>
> At least for some new Go endpoints I'm working on, I'm going to make
> the client decode the json responses, but I think we should definitely
> consider making that the standard with our API clients.
>
> -Rawlin
>
> On Wed, Apr 25, 2018 at 7:14 PM, Schmidt, Andrew (Contractor)
> <An...@comcast.com> wrote:
>> I've noticed that the Put/Post responses are incorrect from some of the Perl endpoints. I'm guessing maybe that is the reason. For example the Put response returns the unmodified record. You have to do a Get to verify that the change occurred.
>>
>> Andy
>>
>> On 4/25/18, 3:53 PM, "Rawlin Peters" <ra...@gmail.com> wrote:
>>
>>     Hey folks,
>>
>>     I noticed in our TO go client that we aren't decoding the JSON
>>     response returned from PUT/POST endpoints. If we actually decoded
>>     those responses, it would be quicker and more useful from a user's
>>     perspective, save bandwidth from all the unnecessary GETs after
>>     POST/PUTs, and also reduce load on the API server.
>>
>>     Is there a reason why we don't decode those responses?
>>
>>     - Rawlin
>>
>>
>>

Re: Traffic Ops go client responses

Posted by Rawlin Peters <ra...@gmail.com>.
If you've found some Perl endpoints that aren't returning the modified
record, I think that's definitely a bug, and there should be GitHub
issues filed to fix them (unless they're already fixed in the Go
implementation). The API should always return the modified resource,
and the client should have immediate access to the modified resource
without needing a subsequent GET after every PUT/POST.

At least for some new Go endpoints I'm working on, I'm going to make
the client decode the json responses, but I think we should definitely
consider making that the standard with our API clients.

-Rawlin

On Wed, Apr 25, 2018 at 7:14 PM, Schmidt, Andrew (Contractor)
<An...@comcast.com> wrote:
> I've noticed that the Put/Post responses are incorrect from some of the Perl endpoints. I'm guessing maybe that is the reason. For example the Put response returns the unmodified record. You have to do a Get to verify that the change occurred.
>
> Andy
>
> On 4/25/18, 3:53 PM, "Rawlin Peters" <ra...@gmail.com> wrote:
>
>     Hey folks,
>
>     I noticed in our TO go client that we aren't decoding the JSON
>     response returned from PUT/POST endpoints. If we actually decoded
>     those responses, it would be quicker and more useful from a user's
>     perspective, save bandwidth from all the unnecessary GETs after
>     POST/PUTs, and also reduce load on the API server.
>
>     Is there a reason why we don't decode those responses?
>
>     - Rawlin
>
>
>

Re: Traffic Ops go client responses

Posted by "Schmidt, Andrew (Contractor)" <An...@comcast.com>.
I've noticed that the Put/Post responses are incorrect from some of the Perl endpoints. I'm guessing maybe that is the reason. For example the Put response returns the unmodified record. You have to do a Get to verify that the change occurred. 

Andy

On 4/25/18, 3:53 PM, "Rawlin Peters" <ra...@gmail.com> wrote:

    Hey folks,
    
    I noticed in our TO go client that we aren't decoding the JSON
    response returned from PUT/POST endpoints. If we actually decoded
    those responses, it would be quicker and more useful from a user's
    perspective, save bandwidth from all the unnecessary GETs after
    POST/PUTs, and also reduce load on the API server.
    
    Is there a reason why we don't decode those responses?
    
    - Rawlin