The 'blob'-iness of ExecutorConfig is intentional so that we can support alternative executors. I'd hate for that to go away.
On Mon, Sep 14, 2015 at 8:56 PM, Jake Farrell <[email protected]> wrote: > This is one of the hoops encountered when using the Thrift api directly and > not using the client, I'd love to see ExecutorConfig.data move to a thrift > object and not be a string blob > > -Jake > > On Mon, Sep 14, 2015 at 9:28 PM, Bill Farner <[email protected]> wrote: > > > I like the idea of adding this API, but i don't see why it requires us to > > make assumptions about ExecutorConfig.data. I hope this doesn't mean we > > would be returning a textual representation of a diff. Can you elaborate > > on that? > > > > On Mon, Sep 14, 2015 at 4:14 PM, Maxim Khutornenko <[email protected]> > > wrote: > > > > > Problem: > > > We currently don't have a good user experience around "aurora job > > > diff" command. The task configs are dumped as "prettified" JSON > > > strings and diffed with the system diff tool. Anyone who tried to use > > > it knows it can be very hard to read especially when it comes to > > > executor data deltas. Also, the implementation is done completely > > > within the Aurora client making it hard to reuse this feature by other > > > clients (e.g.: an external deploy coordination tool). > > > > > > Proposal: > > > Move the diff logic to the scheduler and expose it via a new > > > jobConfigDiff thrift API. > > > > > > Benefits: > > > - Client will no longer have the custom non-reusable logic moving us > > > closer towards a "thin client" goal. > > > - The new RPC can be fully used by any existing or new API clients. > > > - The diff output will be improved via leveraging third party POJO > > > and/or JSON diff libraries [1,2,3, etc.]. > > > - The server updater can be partially/fully unified with the new diff > > > logic further improving the overall DRY-ness. > > > > > > Concerns: > > > - The executor data is currently treated as an opaque string blob on > > > the scheduler side. In reality, it's almost guaranteed to be JSON. In > > > order to deliver the best UX, the scheduler would have to start > > > requiring ExecutorConfig.data to be a valid JSON. > > > > > > Any other concerns/objections/comments? I would like to formalize the > > > proposal be EOW if we reach consensus quickly. > > > > > > Thanks, > > > Maxim > > > > > > [1] - > > > > > > http://java-object-diff.readthedocs.org/en/latest/getting-started/#getting-started > > > [2] - http://javers.org/documentation/diff-examples/ > > > [3] - https://github.com/skyscreamer/JSONassert > > > > > >
