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
