aurora-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bill Farner <wfar...@apache.org>
Subject Re: [RFC] REST / thrift & AURORA-987
Date Tue, 01 Dec 2015 20:24:49 GMT
>
> Well, just to give additional context to that ticket. The ticket mentions
> /apibeta which was literally just a HTTP + TSimpleJSON translation servlet
> on top of the Thrift API that Bill wrote, and it also mentions in the
> description that an alternative would be unlikely to be built on top of
> Thrift. Maybe Bill can chime in with his reasons for writing that?


The thought process here was that a direct thrift->REST translation would
not afford standard ergonomics expected of REST APIs.  For example, things
like meaningful use of HTTP methods, and varied response types based on the
request would be awkward at best to try to bolt on a thrift IDL.


On Tue, Dec 1, 2015 at 11:03 AM, John Sirois <john@conductant.com> wrote:

> On Tue, Dec 1, 2015 at 12:00 PM, David McLaughlin <dmclaughlin@apache.org>
> wrote:
>
> > Well, just to give additional context to that ticket. The ticket mentions
> > /apibeta which was literally just a HTTP + TSimpleJSON translation
> servlet
> > on top of the Thrift API that Bill wrote, and it also mentions in the
> > description that an alternative would be unlikely to be built on top of
> > Thrift. Maybe Bill can chime in with his reasons for writing that?
> >
> > I'm actually struggling to recall a lot of the issues right now.. I
> guess I
> > have Stockholm Syndrome with the Thrift API at this point :)
> >
> > But a good example of some of the workarounds we've had to do with the
> > existing Thrift API can be found if you follow the trail from api.thrift:
> >
> >
> >
> https://github.com/apache/aurora/blob/master/api/src/main/thrift/org/apache/aurora/gen/api.thrift#L977
> >
> > Which leads to https://issues.apache.org/jira/browse/AURORA-541 and
> > https://issues.apache.org/jira/browse/AURORA-539
> >
> > Which leads to some of the comments in this RB:
> >
> > https://reviews.apache.org/r/22790/
> >
> > In that specific case it would have been easier to just pass a query
> > parameter to control the shape of the response rather than have two
> > separate endpoints for performance reasons. I guess my suggestion is to
> > focus less on specific use cases (which the Thrift API is a collection
> of)
> > and just take our core entities (role, environment, job, task, update,
> > etc.) and design a REST API from first principals to query those.
> >
> > Again, I'm not really casting judgement on your implementation proposal
> or
> > how we roll out the new API (I have little to no opinion on this), but
> > simply that I'd like to see us cast a wide net and look at modern APIs
> > (elastic search, etc.) and other 'competitors' in the service scheduling
> > space and how their APIs work for inspiration. From there, we propose an
> > API and after that we have this sort of discussion. Does that make sense?
> >
>
> Totally makes sense.  I was attacking the problem based on my current
> strengths, which favored fixing the glue and worrying about the shape as a
> detail after.  The big assumption being the core entities were in fact
> modeled well and ~correctly by the existing thrift enums, structs  and
> unions.
>
>
> > On Tue, Dec 1, 2015 at 10:23 AM, John Sirois <john@conductant.com>
> wrote:
> >
> > > On Tue, Dec 1, 2015 at 11:21 AM, John Sirois <john@conductant.com>
> > wrote:
> > >
> > > >
> > > >
> > > > On Tue, Dec 1, 2015 at 11:17 AM, Igor Morozov <igmorv@gmail.com>
> > wrote:
> > > >
> > > >> We had very similar concerns here at Uber in regards of Thrift and
> > > >> newer REST API that is coming to Aurora scheduler. It feels like
> > > >> having a general API model in a scheduler and providing whatever
> > > >> interface is necessary/convenient for integration would generally
> be a
> > > >> better option then building REST API layer on top of Thrift API.
> > > >>
> > > >
> > > > To be clear, my intent was to build on top in phase 1, then back out
> > the
> > > > thrift API and gut it as phase 2, then evolve in phase 3.
> > > >
> > > > That said, its clear from both your comment and David's that there
> is a
> > > > desire to go straight to a new API side-by side with the existing API
> > > 1st,
> > > > then transition clients, then gut thrift.
> > > >
> > >
> > > ... I guess those phasings are similar- the key difference is when the
> > new
> > > API is published / blessed.  My original plan was to do this at the end
> > of
> > > phase 3, you two are suggesting do this at the end of phase 1.
> > >
> > >
> > > > Igor, if you also have any specifics on problematic bits of the
> current
> > > > API - I'd love to have those.
> > > >
> > > >
> > > >>
> > > >> On Tue, Dec 1, 2015 at 9:37 AM, David McLaughlin <
> > > dmclaughlin@apache.org>
> > > >> wrote:
> > > >> > Shouldn't we start with the design of the API itself? Won't that
> > > >> influence
> > > >> > many of the answers to these questions?
> > > >> >
> > > >> > E.g. if you're just looking to port the Thrift API 1:1 to a JSON
+
> > > HTTP
> > > >> > interface then that's a very different set of requirements to
> > starting
> > > >> > fresh and doing a better job with our API.
> > > >> >
> > > >> > Personally I don't think the existing Thrift API is a very good
> base
> > > to
> > > >> > build an API on top off. A lot of the endpoints are fit for one
> > > purpose
> > > >> > (e.g. a specific UI view or client function) rather than being
> > > >> flexible. I
> > > >> > can't tell you how many times we wanted to go in and improve
the
> UI
> > in
> > > >> some
> > > >> > way only to find the existing API does not give us access to
the
> > data
> > > we
> > > >> > want.
> > > >> >
> > > >> > So yeah, I feel like the API should be more generic with regards
> to
> > > data
> > > >> > access. So fewer, more-powerful endpoints that support complex
> > > queries.
> > > >> >
> > > >> > On Mon, Nov 30, 2015 at 12:16 PM, John Sirois <
> > john.sirois@gmail.com>
> > > >> wrote:
> > > >> >
> > > >> >> I’ve experimenting on
> > > https://issues.apache.org/jira/browse/AURORA-987
> > > >> for
> > > >> >> the past few weeks and I’d like to ask for feedback on
the
> > direction
> > > >> I’d
> > > >> >> like to head. If you’re interested in the evolution of
the Aurora
> > > REST
> > > >> api,
> > > >> >> read on.
> > > >> >> ------------------------------
> > > >> >>
> > > >> >> AURORA-987 aims to create a first-class REST-like scheduler
> > > interface.
> > > >> I’ve
> > > >> >> re-familiarized myself with the codebase and come to the
> conclusion
> > > >> that
> > > >> >> transitioning to a 1st class REST api requires maintaining
the
> core
> > > >> thrift
> > > >> >> API as the 1st class API until the point the REST API is
fully
> > > >> established
> > > >> >> and clients can all be transitioned.
> > > >> >>
> > > >> >> I think this conclusion is probably uncontroversial, but
the key
> > > >> factors
> > > >> >> pushing this way are:
> > > >> >>
> > > >> >>    1.
> > > >> >>
> > > >> >>    The thrift API has both wide and deep dependencies inside
the
> > > Aurora
> > > >> >>    codebase - 276 imports across 97 files:
> > > >> >>
> > > >> >>    $ git grep "import org.apache.aurora.gen" -- src/main/java/
|
> > grep
> > > >> >> -v "import org.apache.aurora.gen.storage" | wc -l
> > > >> >>    276
> > > >> >>    $ git grep "import org.apache.aurora.gen" -- src/main/java/
|
> > grep
> > > >> >> -v "import org.apache.aurora.gen.storage" | cut -d: -f1 |
sort
> -u |
> > > wc
> > > >> >> -l
> > > >> >>    97
> > > >> >>
> > > >> >>    2.
> > > >> >>
> > > >> >>    The thrift API is stored long-term in the log in serialized
> > form.
> > > >> >>
> > > >> >> Both 1 & 2 dictate that the thrift API, at least its
enums,
> structs
> > > and
> > > >> >> unions, must be maintained for the forseeable future.
> > > >> >> We also have the RPC API (thrift services) - which is currently
a
> > > >> ~thin,
> > > >> >> but not insignificant, container of API processing logic.
For
> > > example,
> > > >> see
> > > >> >> here
> > > >> >> <
> > > >> >>
> > > >>
> > >
> >
> https://github.com/apache/aurora/blob/master/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java#L220-L267
> > > >> >> >
> > > >> >> .
> > > >> >>
> > > >> >> As such it seems to me the REST API should call into the
existing
> > > >> thrift
> > > >> >> API to provide a stable transition and confidence in core
logic
> of
> > > API
> > > >> >> method implementations.
> > > >> >>
> > > >> >> This leads to the following ideas for paths forward:
> > > >> >>
> > > >> >>    1. Hand construct a REST forwarding layer and maintain
it in
> > > tandem
> > > >> with
> > > >> >>    thrift API changes.
> > > >> >>    2. Automate 1 such that thrift API changes cause REST
API
> > changes
> > > >> >>    automatically.
> > > >> >>
> > > >> >> The hand construction path has the obvious maintenance issues,
> but
> > is
> > > >> >> otherwise straight-forward. The maintenance issues should
not be
> > > >> >> overstated, since good tests and some extra review vigilance
> could
> > be
> > > >> >> enough to make the approach work for the period of time both
APIs
> > are
> > > >> >> supported.
> > > >> >>
> > > >> >> That said, an automated solution with a single source of
truth
> for
> > > the
> > > >> API
> > > >> >> definition is clearly preferrable given the automation is
free.
> > > >> >> The automation is far from free though and so I’ve started
> > > >> investigating
> > > >> >> one approach to this automation to flesh out the scope.
> > > >> >>
> > > >> >> We already do our own thrift codegen
> > > >> >> <
> > > >> >>
> > > >>
> > >
> >
> https://github.com/apache/aurora/blob/master/src/main/python/apache/aurora/tools/java/thrift_wrapper_codegen.py
> > > >> >> >
> > > >> >> via a custom gradle ThriftEntitiesPlugin
> > > >> >> <
> > > >> >>
> > > >>
> > >
> >
> https://github.com/apache/aurora/blob/master/buildSrc/src/main/groovy/org/apache/aurora/build/ThriftEntitiesPlugin.groovy
> > > >> >> >
> > > >> >> that works around Apache thrift’s java codegen in order
to
> generate
> > > >> >> immutable wrapper entities for the storage system.
> > > >> >> I propose taking this further and generating our own thrift
API
> and
> > > >> >> entities in 1 pass through our thrift files. These would
be
> > immutable
> > > >> >> thrift entities 1st class with builders for modification
and the
> > > >> entities
> > > >> >> and the generated service interfaces would carry extra metadata
> in
> > > the
> > > >> form
> > > >> >> of annotations to bind REST services and their metadata with.
> > > >> >>
> > > >> >> There are 2 paths I’ve considered towards this end:
> > > >> >>
> > > >> >>    1. Modify Apache thrift to support immutable-style java
output
> > > with
> > > >> >>    support for thrift annotations.
> > > >> >>    2. Write our own thrift parser and code generator to do
said
> > same.
> > > >> >>
> > > >> >> I’ve been pursuing option 2 even though it sounds worse
on its
> > face.
> > > >> The
> > > >> >> swift <https://github.com/facebook/swift> project from
Facebook
> > > brings
> > > >> >> options 1 and 2 back on the same level of undertaking since
the
> > > >> parsing and
> > > >> >> protocol implementations can be leveraged as libraries and
only
> the
> > > >> codegen
> > > >> >> portion need be undertaken (You can see some of that work
here
> > > >> >> <
> > > >> >>
> > > >>
> > >
> >
> https://github.com/apache/aurora/compare/master...jsirois:jsirois/issues/AURORA-987/experiments
> > > >> >> >
> > > >> >> ).
> > > >> >>
> > > >> >> So, with that background 2 questions of the same form:
> > > >> >>
> > > >> >>    1. Is there some other fundamental approach I’m missing
to
> > bolting
> > > >> on a
> > > >> >>    1st class REST API, or is the hand construction approach
> > > favorable?
> > > >> >>    2. Is the approach to single point of API control using
swift
> > > >> misguided?
> > > >> >>    Should I be focusing on Apache thrift enhancement instead?
> > Should
> > > I
> > > >> be
> > > >> >>    generating the *.thrift files instead from a new 1st class
> > source
> > > of
> > > >> >> truth
> > > >> >>    in the form of a json api schema?
> > > >> >>
> > > >> >> Any and all feedback is welcome!
> > > >> >>
> > > >> >>
> > > >>
> > > >>
> > > >>
> > > >> --
> > > >> -Igor
> > > >>
> > > >
> > > >
> > > >
> > > > --
> > > > John Sirois
> > > > 303-512-3301
> > > >
> > >
> > >
> > >
> > > --
> > > John Sirois
> > > 303-512-3301
> > >
> >
>
>
>
> --
> John Sirois
> 303-512-3301
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message