aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Farner" <wfar...@apache.org>
Subject Re: Review Request 29117: Adding thrift API changes document.
Date Wed, 21 Jan 2015 23:43:35 GMT


> On Jan. 9, 2015, 6:44 p.m., Bill Farner wrote:
> > docs/thrift-deprecation.md, line 31
> > <https://reviews.apache.org/r/29117/diff/2/?file=793344#file793344line31>
> >
> >     There should be an item about logging and signaling in API responses when deprecated
fields are used.
> 
> Maxim Khutornenko wrote:
>     We have not done this before and there is no standard way to accomplish it across
our codebases. Should it rather be done at the feature level rather than the thrift schema
level?

I've definitely pushed for server-side logging in the scheduler, client-side logging, and
i definitely think the API should include a `ResponseDetail` entry when a deprecated feature
is used in a request.


> On Jan. 9, 2015, 6:44 p.m., Bill Farner wrote:
> > docs/thrift-deprecation.md, line 3
> > <https://reviews.apache.org/r/29117/diff/2/?file=793344#file793344line3>
> >
> >     First suggestion should be to go read this page: http://diwakergupta.github.io/thrift-missing-guide/
> >     
> >     The page doesn't call out schema evolution, but fills in a bunch of other context.
> 
> Maxim Khutornenko wrote:
>     I had it initially but decided to leave it out to avoid possible confusion. Glad
to add it back if you feel it will be useful.

I see little harm - so i think it should be here.


> On Jan. 9, 2015, 6:44 p.m., Bill Farner wrote:
> > docs/thrift-deprecation.md, line 16
> > <https://reviews.apache.org/r/29117/diff/2/?file=793344#file793344line16>
> >
> >     I have mixed feelings about this.  It's fine on the wire _for specific thrift
encodings_.  This would, for example, break /apibeta.
> 
> Maxim Khutornenko wrote:
>     Do you mean backwards compatibility with he old way of consuming /apibeta output?
I don't think we are at the position to fully support /apibeta compatibilty and given its
rather experimental status we most likely would not do it anyway.

I dunno, i think it's something we should not take so lightly as to encourage casual renames.
 It's a practice we need to begin stepping up anyhow as we prepare to introduce a non-thrift
API.


- Bill


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29117/#review67471
-----------------------------------------------------------


On Jan. 21, 2015, 10:59 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29117/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 10:59 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-973
>     https://issues.apache.org/jira/browse/AURORA-973
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is a first stab at documenting thrift deprecation. Any suggestions/comments are
welcome.
> 
> 
> Diffs
> -----
> 
>   docs/developing-aurora-client.md b9912bce44d65ddd7f1e35f0ea9356a89d5fe767 
>   docs/developing-aurora-scheduler.md 7f6cc2e6c8e01115a9b7a7dc7633bcd88ba02a0f 
>   docs/thrift-deprecation.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29117/diff/
> 
> 
> Testing
> -------
> 
> https://github.com/maxim111333/incubator-aurora/blob/populated_deprecation/docs/thrift-deprecation.md
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


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