aurora-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From John Sirois <j...@conductant.com>
Subject Re: [RFC] REST / thrift & AURORA-987
Date Tue, 01 Dec 2015 20:41:09 GMT
On Tue, Dec 1, 2015 at 1:37 PM, John Sirois <john@conductant.com> wrote:

>
>
> On Tue, Dec 1, 2015 at 1:24 PM, Bill Farner <wfarner@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?
>>
>>
>> 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.
>>
>
> Response types would be dealt with through thrift annotations, ie:
>
> Response getJobs(1: string ownerRole) (method="GET")
>
> Response scheduleCronJob(
>       1: JobConfiguration description (authorizing="true"),
>       3: Lock lock)  (method="POST")
>
> And union response types would be dealt with similarly via - using swagger
> which was my spike target:
> api.thrift: Response getJobs(1: string ownerRole) (method="GET",
> returns="GetJobsResult")
> swagger schema aid:
> https://github.com/swagger-api/swagger-core/blob/master/modules/swagger-annotations/src/main/java/io/swagger/annotations/ApiResponses.java
>

These examples may make it more clear where I was headed.  The API would be
shape preserving, but support REST naturally otherwise - auth, methods,
schema.
Where my idea breaks down is exactly where David pointed out his concern -
if the existing methods are the wrong shape - this doesn't work.

One possibility on that front is to add new thrift methods and restrict the
set of thrift methods exported via the new REST api via another annotation
(REST="true).



>
>
>
>>
>> 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
>> >
>>
>
>
>
> --
> John Sirois
> 303-512-3301
>



-- 
John Sirois
303-512-3301

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