On Tue, Dec 1, 2015 at 1:24 PM, Bill Farner <wfar...@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
>
> On Tue, Dec 1, 2015 at 11:03 AM, John Sirois <j...@conductant.com> wrote:
>
> > On Tue, Dec 1, 2015 at 12:00 PM, David McLaughlin <
> dmclaugh...@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 <j...@conductant.com>
> > wrote:
> > >
> > > > On Tue, Dec 1, 2015 at 11:21 AM, John Sirois <j...@conductant.com>
> > > wrote:
> > > >
> > > > >
> > > > >
> > > > > On Tue, Dec 1, 2015 at 11:17 AM, Igor Morozov <igm...@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 <
> > > > dmclaugh...@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.sir...@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
