I am fine if we move in this direction, but I think we should be very
careful not to break existing functionality.  We need to make sure that if
we do change from path params to query params that we update our API
versions, documentation, and tests accordingly.
Unless I am reading it wrong, it looks like the PR mentioned above by
Jeremy is not updating the version, the docs, or any associated tests.

Thanks,
Dave

On Fri, Apr 6, 2018 at 3:14 PM, Jeremy Mitchell <mitchell...@gmail.com>
wrote:

> Rawlin,
>
> I have submitted a PR to change some new ds request routes to utilize query
> params instead of path/route params (the legacy format):
>
> https://github.com/apache/incubator-trafficcontrol/pull/2094
>
> On Fri, Apr 6, 2018 at 11:59 AM, Rawlin Peters <rawlin.pet...@gmail.com>
> wrote:
>
> > Do we currently have an example of an API endpoint written in the
> > traffic_ops_golang framework that uses only query parameters like
> > this? How would it compare to the legacy format?
> >
> > -Rawlin
> >
> > On Thu, Apr 5, 2018 at 9:45 AM, Dewayne Richardson <dewr...@apache.org>
> > wrote:
> > > Thank you John for giving us "API Use Cases" to think about with these
> > new
> > > TO API Guidelines.  The main goal for these changes are to build TO
> API's
> > > that are intuitive.  I'm of the opinion that if the API's are intuitive
> > > (with easy to understand patterns) that prevents me from wasting time
> > > looking up API docs.  A nice side effect of having simple standards and
> > > patterns is that simplicity ripples into our API Go code which allows
> us
> > to
> > > easily build frameworks that do all of the work instead of the API
> > > snowflakes that we have today.
> > >
> > > I encourage everyone to shoot as many holes into our thoughts around
> this
> > > new direction so that we can see the bigger picture.
> > >
> > > -Dew
> > >
> > > On Wed, Apr 4, 2018 at 10:29 PM, John Rushford <jjrushf...@gmail.com>
> > wrote:
> > >
> > >> Why the change?  It’s my understanding that path parameters should be
> > used
> > >> to specify a particular resource
> > >> and query parameters should be used to sort/filter the query.  Why
> use a
> > >> query parameter to specify a particular
> > >> resource?  Is this REST API best practice?
> > >>
> > >> What about sub resource queries such as using the following:
> > >>
> > >> GET api/1.3/deliveryservices/{xmlID}/urisigning
> > >>
> > >> where you are requesting a particular urisigning keys sub resource for
> > the
> > >> particular deliveryservice resource. You can make it work
> > >> with an xmlid query parameter but what do you return if the query
> > >> parameter is left off, all uri signing keys?  Is that useful?
> > >>
> > >> John
> > >>
> > >> > On Apr 4, 2018, at 3:23 PM, Jeremy Mitchell <mitchell...@gmail.com>
> > >> wrote:
> > >> >
> > >> > tbh i'm not sure about versioning. I was just trying to suggest that
> > new
> > >> > routes be formulated this way per the new API guidelines:
> > >> >
> > >> > GET /foos[?id, name, etc=]
> > >> > POST /foos
> > >> > PUT /foos [?id, name, etc=]
> > >> > DELETE /foos [?id, name, etc=]
> > >> >
> > >> > instead of the old way:
> > >> >
> > >> > GET /foos
> > >> > GET /foos/:id
> > >> > POST /foos
> > >> > PUT /foos/:id
> > >> > DELETE /foos/:id
> > >> >
> > >> > The difference being the use of query params over route/path params.
> > >> >
> > >> > Technically, adding new routes does not break old stuff right so i
> > don't
> > >> > think that warrants a major version roll.
> > >> >
> > >> > While we're on the subject, what does everyone think if we took this
> > one
> > >> > step further and made routes handle a request payload with one or
> more
> > >> > items. For example:
> > >> >
> > >> > GET /foos[?id, name, etc=]
> > >> > POST /foos <-- takes in an array of foos to create
> > >> > PUT /foos <-- takes in an array of foos to update
> > >> > DELETE /foos <-- takes in an array of foos to delete
> > >> >
> > >> > in this scenario, query params only pertain to the GET. The POST,
> PUT
> > and
> > >> > DELETE rely on the contents of the request json...
> > >> >
> > >> > Jeremy
> > >> >
> > >> >
> > >> >
> > >> >
> > >> >
> > >> >
> > >> >
> > >> >
> > >> >
> > >> >
> > >> >
> > >> > On Wed, Apr 4, 2018 at 1:55 PM, Robert Butts <
> > robert.o.bu...@gmail.com>
> > >> > wrote:
> > >> >
> > >> >> That document doesn't mention versions, 1.2 vs 1.3 vs 2.0.
> > >> >>
> > >> >> Just to clarify, changing to query parameters breaks compatibility
> > with
> > >> 1.2
> > >> >> and older, so new APIs in that format have to be a new major
> version,
> > >> i.e.
> > >> >> 2.0, per Semantic Versioning, right?
> > >> >>
> > >> >> On Tue, Apr 3, 2018 at 3:26 PM, Jeremy Mitchell <
> > mitchell...@apache.org
> > >> >
> > >> >> wrote:
> > >> >>
> > >> >>> FYI - I've updated the TO API guidelines to reflect our desire to
> > move
> > >> >> away
> > >> >>> from route/path params and embrace query params in the Golang API.
> > >> >>>
> > >> >>> https://cwiki.apache.org/confluence/display/TC/API+Guidelines
> > >> >>>
> > >> >>> Jeremy
> > >> >>>
> > >> >>
> > >>
> > >>
> >
>

Reply via email to