You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tez.apache.org by Andre Kelpe <ak...@concurrentinc.com> on 2015/08/18 17:01:53 UTC

api compatibility within a minor release

Hi,

I have found a small API incompatibility in the Tez 0.6.2 release. The
DAGClientTimelineImpl constructor got a new parameter for time-outs. This
was not present in 0.6.1 and from a semantic versioning point of view, that
is problematic. I know that the class is marked with the @Private
annotation, but it would be great if such incompatible things aren't
introduced in a bug-fix release. It would be easier for downsteam projects,
if you just added a second constructor.

Is that an oversight or are classes with the @Private annotation subject to
change and dowstream projects simply have to deal with it?

Thanks!

- André

-- 
André Kelpe
andre@concurrentinc.com
http://concurrentinc.com

Re: api compatibility within a minor release

Posted by Siddharth Seth <ss...@apache.org>.
I think there's a jira for this already. I'm not sure it needs to be a
separate library in itself - since the DAGClient already provides
monitoring information, counters etc for Vertices.

On Fri, Dec 11, 2015 at 1:16 PM, Bikas Saha <bi...@apache.org> wrote:

> It could be used by DAGClient internally but essentially it’s an
> additional library that can be used to get detailed information about any
> dag - running or completed (vs DAGClient that interacts with the dag is
> submitted). To be clear, I am calling it TezATSClient in provide clarity as
> to what its doing for the context of this thread. It will hide the details
> of ATS from users and provide a Tez-centric view (independent of internal
> details). Hence the actual name of the client will not have ATS in it.
>
> If that makes sense, should we create jiras following that path?
>
> Bikas
>
> -----Original Message-----
> From: Siddharth Seth [mailto:sseth@apache.org]
> Sent: Thursday, December 10, 2015 8:11 PM
> To: dev@tez.apache.org
> Subject: Re: api compatibility within a minor release
>
> Agree with Andre's point - users do not need to bother about how the data
> is stored and retrieved, as long as it's accessible. Users should not need
> to know details of ATS - or even about it's existence ideally. The
> DAGClient monitors a dag for execution, and provides vertex progress. In
> addition, it should provide task progress and counters. Users should not
> need to worry about where this information comes from - the AM / ATS / some
> other source.
>
> @Bikas, the TezATSClient that you mention - is that meant to be used
> internally by the DAGClient, or is that a replacement to the DAGClient or
> is it an additional library that users will have to use or something else ?
>
>
> On Thu, Dec 10, 2015 at 12:59 PM, Bikas Saha <bi...@apache.org> wrote:
>
> > Yes. That is what I was trying to convey with a TezATSClient - it
> > gives a Tez facade that provides expected semantics like
> > getFooForVertex() etc. and hides the actual ATS details from the user.
> >
> > -----Original Message-----
> > From: Andre Kelpe [mailto:akelpe@concurrentinc.com]
> > Sent: Thursday, December 10, 2015 2:09 AM
> > To: dev@tez.apache.org
> > Subject: Re: api compatibility within a minor release
> >
> > From our point of view ATS is an implementation detail. As you said in
> > Budapest, it changes all the time anyway, so it is not the API we
> > would like to talk to. We care about the information stored in the ATS
> > that is relevant for the current DAG, but we don't really care about
> > the fact that the ATS is used. In MapReduce you also don't know, that
> > the information is stored int he JobHistoryServer. There is an API on
> > the RunningJob class to get to that information. We thing something
> > along those lines would be good. Then you can change the DAGClient and
> > the storage for the information all you want, we as consumers never have
> to think about it.
> >
> > - André
> >
> > On Wed, Dec 9, 2015 at 11:39 PM, Bikas Saha <bi...@apache.org> wrote:
> >
> > > Are we basically looking for detailed data that’s stored in the ATS?
> > > Then should we consider creating a TezATSClient that understand Tez
> > > semantics and fetches this information (in general for any DAG)
> > > instead of augmenting DAGClient (that is responsible for submitting
> > > a Tez DAG and monitoring a single DAG)?
> > >
> > > Bikas
> > >
> > > -----Original Message-----
> > > From: Andre Kelpe [mailto:akelpe@concurrentinc.com]
> > > Sent: Wednesday, December 9, 2015 10:32 AM
> > > To: Hitesh Shah <hi...@apache.org>
> > > Cc: dev@tez.apache.org
> > > Subject: Re: api compatibility within a minor release
> > >
> > > Hi,
> > >
> > > (undusting this thread after a while)
> > >
> > > I have opened https://issues.apache.org/jira/browse/TEZ-2981 to work
> > > around the issue. We believe that would be the best way forward.
> > >
> > > - André
> > >
> > > On Tue, Sep 15, 2015 at 11:04 PM, Hitesh Shah <hi...@apache.org>
> wrote:
> > >
> > > > Hi Andre,
> > > >
> > > > I don’t believe I saw a JIRA created for this? Has this issue been
> > > > resolved?
> > > >
> > > > Also, it would be good if you can collate any particular private
> > > > APIs that you are using so that we can be aware of potential
> > > > issues with such
> > > APIs.
> > > >
> > > > thanks
> > > > — Hitesh
> > > >
> > > > On Aug 20, 2015, at 2:10 AM, Andre Kelpe
> > > > <ak...@concurrentinc.com>
> > > wrote:
> > > >
> > > > > Thanks for the answer. We have a work-around for now. I am going
> > > > > to make that inventory and submit a patch that changes the
> > > > > annotations from @Private to @LimitedPrivate.
> > > > >
> > > > > From a semantic versioning point of view, I would still expect
> > > > > no
> > > > breaking
> > > > > changes in a bug fix release, but if the Tez community at large
> > > > > can work with that, we have to accept that, I guess.
> > > > >
> > > > > - André
> > > > >
> > > > > On Tue, Aug 18, 2015 at 8:34 PM, Hitesh Shah <hi...@apache.org>
> > > wrote:
> > > > >
> > > > >> Hello Andre,
> > > > >>
> > > > >> For the most part, @Private is considered internal
> > > > >> implementation and subject to change at any point. In this
> > > > >> case, even more so as it is an *Impl class.
> > > > >>
> > > > >> What we can do is try the following:
> > > > >>    - Look at all the various @Private classes being used by
> > Cascading.
> > > > >>    - See which ones should not be used at all and which ones
> > > > >> can be considered to be a @LimitedPrivate for Cascading.
> > > > >>    - For LimitedPrivate apis, we would then try to be a bit
> > > > >> more careful with respect to changing/breaking these APIs. I
> > > > >> would probably not say
> > > > that
> > > > >> they have will have the same guarantees as @Public/@Stable but
> > > > >> we can
> > > > work
> > > > >> with the Cascading community to handle changes to these APIs in
> > > > >> a
> > > > workable
> > > > >> manner on an ongoing basis.
> > > > >>
> > > > >> As for this API in question, for the short term fix, I guess a
> > > > >> simple approach might be to introduce a backward compatible API
> > > > >> ( I believe one which does not need the timeout param passed in )?
> > > > >> Would you mind
> > > > filing a
> > > > >> jira and hopefully provide a patch too?
> > > > >>
> > > > >> thanks
> > > > >> — Hitesh
> > > > >>
> > > > >> On Aug 18, 2015, at 8:01 AM, Andre Kelpe
> > > > >> <ak...@concurrentinc.com>
> > > > wrote:
> > > > >>
> > > > >>> Hi,
> > > > >>>
> > > > >>> I have found a small API incompatibility in the Tez 0.6.2
> release.
> > > > >>> The DAGClientTimelineImpl constructor got a new parameter for
> > > time-outs.
> > > > This
> > > > >>> was not present in 0.6.1 and from a semantic versioning point
> > > > >>> of view,
> > > > >> that
> > > > >>> is problematic. I know that the class is marked with the
> > > > >>> @Private annotation, but it would be great if such
> > > > >>> incompatible things aren't introduced in a bug-fix release. It
> > > > >>> would be easier for downsteam
> > > > >> projects,
> > > > >>> if you just added a second constructor.
> > > > >>>
> > > > >>> Is that an oversight or are classes with the @Private
> > > > >>> annotation
> > > > subject
> > > > >> to
> > > > >>> change and dowstream projects simply have to deal with it?
> > > > >>>
> > > > >>> Thanks!
> > > > >>>
> > > > >>> - André
> > > > >>>
> > > > >>> --
> > > > >>> André Kelpe
> > > > >>> andre@concurrentinc.com
> > > > >>> http://concurrentinc.com
> > > > >>
> > > > >>
> > > > >
> > > > >
> > > > > --
> > > > > André Kelpe
> > > > > andre@concurrentinc.com
> > > > > http://concurrentinc.com
> > > >
> > > >
> > >
> > >
> > > --
> > > André Kelpe
> > > andre@concurrentinc.com
> > > http://concurrentinc.com
> > >
> > >
> >
> >
> > --
> > André Kelpe
> > andre@concurrentinc.com
> > http://concurrentinc.com
> >
> >
>
>

RE: api compatibility within a minor release

Posted by Bikas Saha <bi...@apache.org>.
It could be used by DAGClient internally but essentially it’s an additional library that can be used to get detailed information about any dag - running or completed (vs DAGClient that interacts with the dag is submitted). To be clear, I am calling it TezATSClient in provide clarity as to what its doing for the context of this thread. It will hide the details of ATS from users and provide a Tez-centric view (independent of internal details). Hence the actual name of the client will not have ATS in it.

If that makes sense, should we create jiras following that path?

Bikas

-----Original Message-----
From: Siddharth Seth [mailto:sseth@apache.org] 
Sent: Thursday, December 10, 2015 8:11 PM
To: dev@tez.apache.org
Subject: Re: api compatibility within a minor release

Agree with Andre's point - users do not need to bother about how the data is stored and retrieved, as long as it's accessible. Users should not need to know details of ATS - or even about it's existence ideally. The DAGClient monitors a dag for execution, and provides vertex progress. In addition, it should provide task progress and counters. Users should not need to worry about where this information comes from - the AM / ATS / some other source.

@Bikas, the TezATSClient that you mention - is that meant to be used internally by the DAGClient, or is that a replacement to the DAGClient or is it an additional library that users will have to use or something else ?


On Thu, Dec 10, 2015 at 12:59 PM, Bikas Saha <bi...@apache.org> wrote:

> Yes. That is what I was trying to convey with a TezATSClient - it 
> gives a Tez facade that provides expected semantics like 
> getFooForVertex() etc. and hides the actual ATS details from the user.
>
> -----Original Message-----
> From: Andre Kelpe [mailto:akelpe@concurrentinc.com]
> Sent: Thursday, December 10, 2015 2:09 AM
> To: dev@tez.apache.org
> Subject: Re: api compatibility within a minor release
>
> From our point of view ATS is an implementation detail. As you said in 
> Budapest, it changes all the time anyway, so it is not the API we 
> would like to talk to. We care about the information stored in the ATS 
> that is relevant for the current DAG, but we don't really care about 
> the fact that the ATS is used. In MapReduce you also don't know, that 
> the information is stored int he JobHistoryServer. There is an API on 
> the RunningJob class to get to that information. We thing something 
> along those lines would be good. Then you can change the DAGClient and 
> the storage for the information all you want, we as consumers never have to think about it.
>
> - André
>
> On Wed, Dec 9, 2015 at 11:39 PM, Bikas Saha <bi...@apache.org> wrote:
>
> > Are we basically looking for detailed data that’s stored in the ATS?
> > Then should we consider creating a TezATSClient that understand Tez 
> > semantics and fetches this information (in general for any DAG) 
> > instead of augmenting DAGClient (that is responsible for submitting 
> > a Tez DAG and monitoring a single DAG)?
> >
> > Bikas
> >
> > -----Original Message-----
> > From: Andre Kelpe [mailto:akelpe@concurrentinc.com]
> > Sent: Wednesday, December 9, 2015 10:32 AM
> > To: Hitesh Shah <hi...@apache.org>
> > Cc: dev@tez.apache.org
> > Subject: Re: api compatibility within a minor release
> >
> > Hi,
> >
> > (undusting this thread after a while)
> >
> > I have opened https://issues.apache.org/jira/browse/TEZ-2981 to work 
> > around the issue. We believe that would be the best way forward.
> >
> > - André
> >
> > On Tue, Sep 15, 2015 at 11:04 PM, Hitesh Shah <hi...@apache.org> wrote:
> >
> > > Hi Andre,
> > >
> > > I don’t believe I saw a JIRA created for this? Has this issue been 
> > > resolved?
> > >
> > > Also, it would be good if you can collate any particular private 
> > > APIs that you are using so that we can be aware of potential 
> > > issues with such
> > APIs.
> > >
> > > thanks
> > > — Hitesh
> > >
> > > On Aug 20, 2015, at 2:10 AM, Andre Kelpe 
> > > <ak...@concurrentinc.com>
> > wrote:
> > >
> > > > Thanks for the answer. We have a work-around for now. I am going 
> > > > to make that inventory and submit a patch that changes the 
> > > > annotations from @Private to @LimitedPrivate.
> > > >
> > > > From a semantic versioning point of view, I would still expect 
> > > > no
> > > breaking
> > > > changes in a bug fix release, but if the Tez community at large 
> > > > can work with that, we have to accept that, I guess.
> > > >
> > > > - André
> > > >
> > > > On Tue, Aug 18, 2015 at 8:34 PM, Hitesh Shah <hi...@apache.org>
> > wrote:
> > > >
> > > >> Hello Andre,
> > > >>
> > > >> For the most part, @Private is considered internal 
> > > >> implementation and subject to change at any point. In this 
> > > >> case, even more so as it is an *Impl class.
> > > >>
> > > >> What we can do is try the following:
> > > >>    - Look at all the various @Private classes being used by
> Cascading.
> > > >>    - See which ones should not be used at all and which ones 
> > > >> can be considered to be a @LimitedPrivate for Cascading.
> > > >>    - For LimitedPrivate apis, we would then try to be a bit 
> > > >> more careful with respect to changing/breaking these APIs. I 
> > > >> would probably not say
> > > that
> > > >> they have will have the same guarantees as @Public/@Stable but 
> > > >> we can
> > > work
> > > >> with the Cascading community to handle changes to these APIs in 
> > > >> a
> > > workable
> > > >> manner on an ongoing basis.
> > > >>
> > > >> As for this API in question, for the short term fix, I guess a 
> > > >> simple approach might be to introduce a backward compatible API 
> > > >> ( I believe one which does not need the timeout param passed in )?
> > > >> Would you mind
> > > filing a
> > > >> jira and hopefully provide a patch too?
> > > >>
> > > >> thanks
> > > >> — Hitesh
> > > >>
> > > >> On Aug 18, 2015, at 8:01 AM, Andre Kelpe 
> > > >> <ak...@concurrentinc.com>
> > > wrote:
> > > >>
> > > >>> Hi,
> > > >>>
> > > >>> I have found a small API incompatibility in the Tez 0.6.2 release.
> > > >>> The DAGClientTimelineImpl constructor got a new parameter for
> > time-outs.
> > > This
> > > >>> was not present in 0.6.1 and from a semantic versioning point 
> > > >>> of view,
> > > >> that
> > > >>> is problematic. I know that the class is marked with the 
> > > >>> @Private annotation, but it would be great if such 
> > > >>> incompatible things aren't introduced in a bug-fix release. It 
> > > >>> would be easier for downsteam
> > > >> projects,
> > > >>> if you just added a second constructor.
> > > >>>
> > > >>> Is that an oversight or are classes with the @Private 
> > > >>> annotation
> > > subject
> > > >> to
> > > >>> change and dowstream projects simply have to deal with it?
> > > >>>
> > > >>> Thanks!
> > > >>>
> > > >>> - André
> > > >>>
> > > >>> --
> > > >>> André Kelpe
> > > >>> andre@concurrentinc.com
> > > >>> http://concurrentinc.com
> > > >>
> > > >>
> > > >
> > > >
> > > > --
> > > > André Kelpe
> > > > andre@concurrentinc.com
> > > > http://concurrentinc.com
> > >
> > >
> >
> >
> > --
> > André Kelpe
> > andre@concurrentinc.com
> > http://concurrentinc.com
> >
> >
>
>
> --
> André Kelpe
> andre@concurrentinc.com
> http://concurrentinc.com
>
>


Re: api compatibility within a minor release

Posted by Siddharth Seth <ss...@apache.org>.
Agree with Andre's point - users do not need to bother about how the data
is stored and retrieved, as long as it's accessible. Users should not need
to know details of ATS - or even about it's existence ideally. The
DAGClient monitors a dag for execution, and provides vertex progress. In
addition, it should provide task progress and counters. Users should not
need to worry about where this information comes from - the AM / ATS / some
other source.

@Bikas, the TezATSClient that you mention - is that meant to be used
internally by the DAGClient, or is that a replacement to the DAGClient or
is it an additional library that users will have to use or something else ?


On Thu, Dec 10, 2015 at 12:59 PM, Bikas Saha <bi...@apache.org> wrote:

> Yes. That is what I was trying to convey with a TezATSClient - it gives a
> Tez facade that provides expected semantics like getFooForVertex() etc. and
> hides the actual ATS details from the user.
>
> -----Original Message-----
> From: Andre Kelpe [mailto:akelpe@concurrentinc.com]
> Sent: Thursday, December 10, 2015 2:09 AM
> To: dev@tez.apache.org
> Subject: Re: api compatibility within a minor release
>
> From our point of view ATS is an implementation detail. As you said in
> Budapest, it changes all the time anyway, so it is not the API we would
> like to talk to. We care about the information stored in the ATS that is
> relevant for the current DAG, but we don't really care about the fact that
> the ATS is used. In MapReduce you also don't know, that the information is
> stored int he JobHistoryServer. There is an API on the RunningJob class to
> get to that information. We thing something along those lines would be
> good. Then you can change the DAGClient and the storage for the information
> all you want, we as consumers never have to think about it.
>
> - André
>
> On Wed, Dec 9, 2015 at 11:39 PM, Bikas Saha <bi...@apache.org> wrote:
>
> > Are we basically looking for detailed data that’s stored in the ATS?
> > Then should we consider creating a TezATSClient that understand Tez
> > semantics and fetches this information (in general for any DAG)
> > instead of augmenting DAGClient (that is responsible for submitting a
> > Tez DAG and monitoring a single DAG)?
> >
> > Bikas
> >
> > -----Original Message-----
> > From: Andre Kelpe [mailto:akelpe@concurrentinc.com]
> > Sent: Wednesday, December 9, 2015 10:32 AM
> > To: Hitesh Shah <hi...@apache.org>
> > Cc: dev@tez.apache.org
> > Subject: Re: api compatibility within a minor release
> >
> > Hi,
> >
> > (undusting this thread after a while)
> >
> > I have opened https://issues.apache.org/jira/browse/TEZ-2981 to work
> > around the issue. We believe that would be the best way forward.
> >
> > - André
> >
> > On Tue, Sep 15, 2015 at 11:04 PM, Hitesh Shah <hi...@apache.org> wrote:
> >
> > > Hi Andre,
> > >
> > > I don’t believe I saw a JIRA created for this? Has this issue been
> > > resolved?
> > >
> > > Also, it would be good if you can collate any particular private
> > > APIs that you are using so that we can be aware of potential issues
> > > with such
> > APIs.
> > >
> > > thanks
> > > — Hitesh
> > >
> > > On Aug 20, 2015, at 2:10 AM, Andre Kelpe <ak...@concurrentinc.com>
> > wrote:
> > >
> > > > Thanks for the answer. We have a work-around for now. I am going
> > > > to make that inventory and submit a patch that changes the
> > > > annotations from @Private to @LimitedPrivate.
> > > >
> > > > From a semantic versioning point of view, I would still expect no
> > > breaking
> > > > changes in a bug fix release, but if the Tez community at large
> > > > can work with that, we have to accept that, I guess.
> > > >
> > > > - André
> > > >
> > > > On Tue, Aug 18, 2015 at 8:34 PM, Hitesh Shah <hi...@apache.org>
> > wrote:
> > > >
> > > >> Hello Andre,
> > > >>
> > > >> For the most part, @Private is considered internal implementation
> > > >> and subject to change at any point. In this case, even more so as
> > > >> it is an *Impl class.
> > > >>
> > > >> What we can do is try the following:
> > > >>    - Look at all the various @Private classes being used by
> Cascading.
> > > >>    - See which ones should not be used at all and which ones can
> > > >> be considered to be a @LimitedPrivate for Cascading.
> > > >>    - For LimitedPrivate apis, we would then try to be a bit more
> > > >> careful with respect to changing/breaking these APIs. I would
> > > >> probably not say
> > > that
> > > >> they have will have the same guarantees as @Public/@Stable but we
> > > >> can
> > > work
> > > >> with the Cascading community to handle changes to these APIs in a
> > > workable
> > > >> manner on an ongoing basis.
> > > >>
> > > >> As for this API in question, for the short term fix, I guess a
> > > >> simple approach might be to introduce a backward compatible API (
> > > >> I believe one which does not need the timeout param passed in )?
> > > >> Would you mind
> > > filing a
> > > >> jira and hopefully provide a patch too?
> > > >>
> > > >> thanks
> > > >> — Hitesh
> > > >>
> > > >> On Aug 18, 2015, at 8:01 AM, Andre Kelpe
> > > >> <ak...@concurrentinc.com>
> > > wrote:
> > > >>
> > > >>> Hi,
> > > >>>
> > > >>> I have found a small API incompatibility in the Tez 0.6.2 release.
> > > >>> The DAGClientTimelineImpl constructor got a new parameter for
> > time-outs.
> > > This
> > > >>> was not present in 0.6.1 and from a semantic versioning point of
> > > >>> view,
> > > >> that
> > > >>> is problematic. I know that the class is marked with the
> > > >>> @Private annotation, but it would be great if such incompatible
> > > >>> things aren't introduced in a bug-fix release. It would be
> > > >>> easier for downsteam
> > > >> projects,
> > > >>> if you just added a second constructor.
> > > >>>
> > > >>> Is that an oversight or are classes with the @Private annotation
> > > subject
> > > >> to
> > > >>> change and dowstream projects simply have to deal with it?
> > > >>>
> > > >>> Thanks!
> > > >>>
> > > >>> - André
> > > >>>
> > > >>> --
> > > >>> André Kelpe
> > > >>> andre@concurrentinc.com
> > > >>> http://concurrentinc.com
> > > >>
> > > >>
> > > >
> > > >
> > > > --
> > > > André Kelpe
> > > > andre@concurrentinc.com
> > > > http://concurrentinc.com
> > >
> > >
> >
> >
> > --
> > André Kelpe
> > andre@concurrentinc.com
> > http://concurrentinc.com
> >
> >
>
>
> --
> André Kelpe
> andre@concurrentinc.com
> http://concurrentinc.com
>
>

RE: api compatibility within a minor release

Posted by Bikas Saha <bi...@apache.org>.
Yes. That is what I was trying to convey with a TezATSClient - it gives a Tez facade that provides expected semantics like getFooForVertex() etc. and hides the actual ATS details from the user.

-----Original Message-----
From: Andre Kelpe [mailto:akelpe@concurrentinc.com] 
Sent: Thursday, December 10, 2015 2:09 AM
To: dev@tez.apache.org
Subject: Re: api compatibility within a minor release

>From our point of view ATS is an implementation detail. As you said in Budapest, it changes all the time anyway, so it is not the API we would like to talk to. We care about the information stored in the ATS that is relevant for the current DAG, but we don't really care about the fact that the ATS is used. In MapReduce you also don't know, that the information is stored int he JobHistoryServer. There is an API on the RunningJob class to get to that information. We thing something along those lines would be good. Then you can change the DAGClient and the storage for the information all you want, we as consumers never have to think about it.

- André

On Wed, Dec 9, 2015 at 11:39 PM, Bikas Saha <bi...@apache.org> wrote:

> Are we basically looking for detailed data that’s stored in the ATS? 
> Then should we consider creating a TezATSClient that understand Tez 
> semantics and fetches this information (in general for any DAG) 
> instead of augmenting DAGClient (that is responsible for submitting a 
> Tez DAG and monitoring a single DAG)?
>
> Bikas
>
> -----Original Message-----
> From: Andre Kelpe [mailto:akelpe@concurrentinc.com]
> Sent: Wednesday, December 9, 2015 10:32 AM
> To: Hitesh Shah <hi...@apache.org>
> Cc: dev@tez.apache.org
> Subject: Re: api compatibility within a minor release
>
> Hi,
>
> (undusting this thread after a while)
>
> I have opened https://issues.apache.org/jira/browse/TEZ-2981 to work 
> around the issue. We believe that would be the best way forward.
>
> - André
>
> On Tue, Sep 15, 2015 at 11:04 PM, Hitesh Shah <hi...@apache.org> wrote:
>
> > Hi Andre,
> >
> > I don’t believe I saw a JIRA created for this? Has this issue been 
> > resolved?
> >
> > Also, it would be good if you can collate any particular private 
> > APIs that you are using so that we can be aware of potential issues 
> > with such
> APIs.
> >
> > thanks
> > — Hitesh
> >
> > On Aug 20, 2015, at 2:10 AM, Andre Kelpe <ak...@concurrentinc.com>
> wrote:
> >
> > > Thanks for the answer. We have a work-around for now. I am going 
> > > to make that inventory and submit a patch that changes the 
> > > annotations from @Private to @LimitedPrivate.
> > >
> > > From a semantic versioning point of view, I would still expect no
> > breaking
> > > changes in a bug fix release, but if the Tez community at large 
> > > can work with that, we have to accept that, I guess.
> > >
> > > - André
> > >
> > > On Tue, Aug 18, 2015 at 8:34 PM, Hitesh Shah <hi...@apache.org>
> wrote:
> > >
> > >> Hello Andre,
> > >>
> > >> For the most part, @Private is considered internal implementation 
> > >> and subject to change at any point. In this case, even more so as 
> > >> it is an *Impl class.
> > >>
> > >> What we can do is try the following:
> > >>    - Look at all the various @Private classes being used by Cascading.
> > >>    - See which ones should not be used at all and which ones can 
> > >> be considered to be a @LimitedPrivate for Cascading.
> > >>    - For LimitedPrivate apis, we would then try to be a bit more 
> > >> careful with respect to changing/breaking these APIs. I would 
> > >> probably not say
> > that
> > >> they have will have the same guarantees as @Public/@Stable but we 
> > >> can
> > work
> > >> with the Cascading community to handle changes to these APIs in a
> > workable
> > >> manner on an ongoing basis.
> > >>
> > >> As for this API in question, for the short term fix, I guess a 
> > >> simple approach might be to introduce a backward compatible API ( 
> > >> I believe one which does not need the timeout param passed in )?
> > >> Would you mind
> > filing a
> > >> jira and hopefully provide a patch too?
> > >>
> > >> thanks
> > >> — Hitesh
> > >>
> > >> On Aug 18, 2015, at 8:01 AM, Andre Kelpe 
> > >> <ak...@concurrentinc.com>
> > wrote:
> > >>
> > >>> Hi,
> > >>>
> > >>> I have found a small API incompatibility in the Tez 0.6.2 release.
> > >>> The DAGClientTimelineImpl constructor got a new parameter for
> time-outs.
> > This
> > >>> was not present in 0.6.1 and from a semantic versioning point of 
> > >>> view,
> > >> that
> > >>> is problematic. I know that the class is marked with the 
> > >>> @Private annotation, but it would be great if such incompatible 
> > >>> things aren't introduced in a bug-fix release. It would be 
> > >>> easier for downsteam
> > >> projects,
> > >>> if you just added a second constructor.
> > >>>
> > >>> Is that an oversight or are classes with the @Private annotation
> > subject
> > >> to
> > >>> change and dowstream projects simply have to deal with it?
> > >>>
> > >>> Thanks!
> > >>>
> > >>> - André
> > >>>
> > >>> --
> > >>> André Kelpe
> > >>> andre@concurrentinc.com
> > >>> http://concurrentinc.com
> > >>
> > >>
> > >
> > >
> > > --
> > > André Kelpe
> > > andre@concurrentinc.com
> > > http://concurrentinc.com
> >
> >
>
>
> --
> André Kelpe
> andre@concurrentinc.com
> http://concurrentinc.com
>
>


--
André Kelpe
andre@concurrentinc.com
http://concurrentinc.com


Re: api compatibility within a minor release

Posted by Andre Kelpe <ak...@concurrentinc.com>.
>From our point of view ATS is an implementation detail. As you said in
Budapest, it changes all the time anyway, so it is not the API we would
like to talk to. We care about the information stored in the ATS that is
relevant for the current DAG, but we don't really care about the fact that
the ATS is used. In MapReduce you also don't know, that the information is
stored int he JobHistoryServer. There is an API on the RunningJob class to
get to that information. We thing something along those lines would be
good. Then you can change the DAGClient and the storage for the information
all you want, we as consumers never have to think about it.

- André

On Wed, Dec 9, 2015 at 11:39 PM, Bikas Saha <bi...@apache.org> wrote:

> Are we basically looking for detailed data that’s stored in the ATS? Then
> should we consider creating a TezATSClient that understand Tez semantics
> and fetches this information (in general for any DAG) instead of augmenting
> DAGClient (that is responsible for submitting a Tez DAG and monitoring a
> single DAG)?
>
> Bikas
>
> -----Original Message-----
> From: Andre Kelpe [mailto:akelpe@concurrentinc.com]
> Sent: Wednesday, December 9, 2015 10:32 AM
> To: Hitesh Shah <hi...@apache.org>
> Cc: dev@tez.apache.org
> Subject: Re: api compatibility within a minor release
>
> Hi,
>
> (undusting this thread after a while)
>
> I have opened https://issues.apache.org/jira/browse/TEZ-2981 to work
> around the issue. We believe that would be the best way forward.
>
> - André
>
> On Tue, Sep 15, 2015 at 11:04 PM, Hitesh Shah <hi...@apache.org> wrote:
>
> > Hi Andre,
> >
> > I don’t believe I saw a JIRA created for this? Has this issue been
> > resolved?
> >
> > Also, it would be good if you can collate any particular private APIs
> > that you are using so that we can be aware of potential issues with such
> APIs.
> >
> > thanks
> > — Hitesh
> >
> > On Aug 20, 2015, at 2:10 AM, Andre Kelpe <ak...@concurrentinc.com>
> wrote:
> >
> > > Thanks for the answer. We have a work-around for now. I am going to
> > > make that inventory and submit a patch that changes the annotations
> > > from @Private to @LimitedPrivate.
> > >
> > > From a semantic versioning point of view, I would still expect no
> > breaking
> > > changes in a bug fix release, but if the Tez community at large can
> > > work with that, we have to accept that, I guess.
> > >
> > > - André
> > >
> > > On Tue, Aug 18, 2015 at 8:34 PM, Hitesh Shah <hi...@apache.org>
> wrote:
> > >
> > >> Hello Andre,
> > >>
> > >> For the most part, @Private is considered internal implementation
> > >> and subject to change at any point. In this case, even more so as
> > >> it is an *Impl class.
> > >>
> > >> What we can do is try the following:
> > >>    - Look at all the various @Private classes being used by Cascading.
> > >>    - See which ones should not be used at all and which ones can be
> > >> considered to be a @LimitedPrivate for Cascading.
> > >>    - For LimitedPrivate apis, we would then try to be a bit more
> > >> careful with respect to changing/breaking these APIs. I would
> > >> probably not say
> > that
> > >> they have will have the same guarantees as @Public/@Stable but we
> > >> can
> > work
> > >> with the Cascading community to handle changes to these APIs in a
> > workable
> > >> manner on an ongoing basis.
> > >>
> > >> As for this API in question, for the short term fix, I guess a
> > >> simple approach might be to introduce a backward compatible API ( I
> > >> believe one which does not need the timeout param passed in )?
> > >> Would you mind
> > filing a
> > >> jira and hopefully provide a patch too?
> > >>
> > >> thanks
> > >> — Hitesh
> > >>
> > >> On Aug 18, 2015, at 8:01 AM, Andre Kelpe <ak...@concurrentinc.com>
> > wrote:
> > >>
> > >>> Hi,
> > >>>
> > >>> I have found a small API incompatibility in the Tez 0.6.2 release.
> > >>> The DAGClientTimelineImpl constructor got a new parameter for
> time-outs.
> > This
> > >>> was not present in 0.6.1 and from a semantic versioning point of
> > >>> view,
> > >> that
> > >>> is problematic. I know that the class is marked with the @Private
> > >>> annotation, but it would be great if such incompatible things
> > >>> aren't introduced in a bug-fix release. It would be easier for
> > >>> downsteam
> > >> projects,
> > >>> if you just added a second constructor.
> > >>>
> > >>> Is that an oversight or are classes with the @Private annotation
> > subject
> > >> to
> > >>> change and dowstream projects simply have to deal with it?
> > >>>
> > >>> Thanks!
> > >>>
> > >>> - André
> > >>>
> > >>> --
> > >>> André Kelpe
> > >>> andre@concurrentinc.com
> > >>> http://concurrentinc.com
> > >>
> > >>
> > >
> > >
> > > --
> > > André Kelpe
> > > andre@concurrentinc.com
> > > http://concurrentinc.com
> >
> >
>
>
> --
> André Kelpe
> andre@concurrentinc.com
> http://concurrentinc.com
>
>


-- 
André Kelpe
andre@concurrentinc.com
http://concurrentinc.com

RE: api compatibility within a minor release

Posted by Bikas Saha <bi...@apache.org>.
Are we basically looking for detailed data that’s stored in the ATS? Then should we consider creating a TezATSClient that understand Tez semantics and fetches this information (in general for any DAG) instead of augmenting DAGClient (that is responsible for submitting a Tez DAG and monitoring a single DAG)?

Bikas

-----Original Message-----
From: Andre Kelpe [mailto:akelpe@concurrentinc.com] 
Sent: Wednesday, December 9, 2015 10:32 AM
To: Hitesh Shah <hi...@apache.org>
Cc: dev@tez.apache.org
Subject: Re: api compatibility within a minor release

Hi,

(undusting this thread after a while)

I have opened https://issues.apache.org/jira/browse/TEZ-2981 to work around the issue. We believe that would be the best way forward.

- André

On Tue, Sep 15, 2015 at 11:04 PM, Hitesh Shah <hi...@apache.org> wrote:

> Hi Andre,
>
> I don’t believe I saw a JIRA created for this? Has this issue been 
> resolved?
>
> Also, it would be good if you can collate any particular private APIs 
> that you are using so that we can be aware of potential issues with such APIs.
>
> thanks
> — Hitesh
>
> On Aug 20, 2015, at 2:10 AM, Andre Kelpe <ak...@concurrentinc.com> wrote:
>
> > Thanks for the answer. We have a work-around for now. I am going to 
> > make that inventory and submit a patch that changes the annotations 
> > from @Private to @LimitedPrivate.
> >
> > From a semantic versioning point of view, I would still expect no
> breaking
> > changes in a bug fix release, but if the Tez community at large can 
> > work with that, we have to accept that, I guess.
> >
> > - André
> >
> > On Tue, Aug 18, 2015 at 8:34 PM, Hitesh Shah <hi...@apache.org> wrote:
> >
> >> Hello Andre,
> >>
> >> For the most part, @Private is considered internal implementation 
> >> and subject to change at any point. In this case, even more so as 
> >> it is an *Impl class.
> >>
> >> What we can do is try the following:
> >>    - Look at all the various @Private classes being used by Cascading.
> >>    - See which ones should not be used at all and which ones can be 
> >> considered to be a @LimitedPrivate for Cascading.
> >>    - For LimitedPrivate apis, we would then try to be a bit more 
> >> careful with respect to changing/breaking these APIs. I would 
> >> probably not say
> that
> >> they have will have the same guarantees as @Public/@Stable but we 
> >> can
> work
> >> with the Cascading community to handle changes to these APIs in a
> workable
> >> manner on an ongoing basis.
> >>
> >> As for this API in question, for the short term fix, I guess a 
> >> simple approach might be to introduce a backward compatible API ( I 
> >> believe one which does not need the timeout param passed in )? 
> >> Would you mind
> filing a
> >> jira and hopefully provide a patch too?
> >>
> >> thanks
> >> — Hitesh
> >>
> >> On Aug 18, 2015, at 8:01 AM, Andre Kelpe <ak...@concurrentinc.com>
> wrote:
> >>
> >>> Hi,
> >>>
> >>> I have found a small API incompatibility in the Tez 0.6.2 release. 
> >>> The DAGClientTimelineImpl constructor got a new parameter for time-outs.
> This
> >>> was not present in 0.6.1 and from a semantic versioning point of 
> >>> view,
> >> that
> >>> is problematic. I know that the class is marked with the @Private 
> >>> annotation, but it would be great if such incompatible things 
> >>> aren't introduced in a bug-fix release. It would be easier for 
> >>> downsteam
> >> projects,
> >>> if you just added a second constructor.
> >>>
> >>> Is that an oversight or are classes with the @Private annotation
> subject
> >> to
> >>> change and dowstream projects simply have to deal with it?
> >>>
> >>> Thanks!
> >>>
> >>> - André
> >>>
> >>> --
> >>> André Kelpe
> >>> andre@concurrentinc.com
> >>> http://concurrentinc.com
> >>
> >>
> >
> >
> > --
> > André Kelpe
> > andre@concurrentinc.com
> > http://concurrentinc.com
>
>


--
André Kelpe
andre@concurrentinc.com
http://concurrentinc.com


Re: api compatibility within a minor release

Posted by Andre Kelpe <ak...@concurrentinc.com>.
Hi,

(undusting this thread after a while)

I have opened https://issues.apache.org/jira/browse/TEZ-2981 to work around
the issue. We believe that would be the best way forward.

- André

On Tue, Sep 15, 2015 at 11:04 PM, Hitesh Shah <hi...@apache.org> wrote:

> Hi Andre,
>
> I don’t believe I saw a JIRA created for this? Has this issue been
> resolved?
>
> Also, it would be good if you can collate any particular private APIs that
> you are using so that we can be aware of potential issues with such APIs.
>
> thanks
> — Hitesh
>
> On Aug 20, 2015, at 2:10 AM, Andre Kelpe <ak...@concurrentinc.com> wrote:
>
> > Thanks for the answer. We have a work-around for now. I am going to make
> > that inventory and submit a patch that changes the annotations from
> > @Private to @LimitedPrivate.
> >
> > From a semantic versioning point of view, I would still expect no
> breaking
> > changes in a bug fix release, but if the Tez community at large can work
> > with that, we have to accept that, I guess.
> >
> > - André
> >
> > On Tue, Aug 18, 2015 at 8:34 PM, Hitesh Shah <hi...@apache.org> wrote:
> >
> >> Hello Andre,
> >>
> >> For the most part, @Private is considered internal implementation and
> >> subject to change at any point. In this case, even more so as it is an
> >> *Impl class.
> >>
> >> What we can do is try the following:
> >>    - Look at all the various @Private classes being used by Cascading.
> >>    - See which ones should not be used at all and which ones can be
> >> considered to be a @LimitedPrivate for Cascading.
> >>    - For LimitedPrivate apis, we would then try to be a bit more careful
> >> with respect to changing/breaking these APIs. I would probably not say
> that
> >> they have will have the same guarantees as @Public/@Stable but we can
> work
> >> with the Cascading community to handle changes to these APIs in a
> workable
> >> manner on an ongoing basis.
> >>
> >> As for this API in question, for the short term fix, I guess a simple
> >> approach might be to introduce a backward compatible API ( I believe one
> >> which does not need the timeout param passed in )? Would you mind
> filing a
> >> jira and hopefully provide a patch too?
> >>
> >> thanks
> >> — Hitesh
> >>
> >> On Aug 18, 2015, at 8:01 AM, Andre Kelpe <ak...@concurrentinc.com>
> wrote:
> >>
> >>> Hi,
> >>>
> >>> I have found a small API incompatibility in the Tez 0.6.2 release. The
> >>> DAGClientTimelineImpl constructor got a new parameter for time-outs.
> This
> >>> was not present in 0.6.1 and from a semantic versioning point of view,
> >> that
> >>> is problematic. I know that the class is marked with the @Private
> >>> annotation, but it would be great if such incompatible things aren't
> >>> introduced in a bug-fix release. It would be easier for downsteam
> >> projects,
> >>> if you just added a second constructor.
> >>>
> >>> Is that an oversight or are classes with the @Private annotation
> subject
> >> to
> >>> change and dowstream projects simply have to deal with it?
> >>>
> >>> Thanks!
> >>>
> >>> - André
> >>>
> >>> --
> >>> André Kelpe
> >>> andre@concurrentinc.com
> >>> http://concurrentinc.com
> >>
> >>
> >
> >
> > --
> > André Kelpe
> > andre@concurrentinc.com
> > http://concurrentinc.com
>
>


-- 
André Kelpe
andre@concurrentinc.com
http://concurrentinc.com

Re: api compatibility within a minor release

Posted by Hitesh Shah <hi...@apache.org>.
Hi Andre, 

I don’t believe I saw a JIRA created for this? Has this issue been resolved? 

Also, it would be good if you can collate any particular private APIs that you are using so that we can be aware of potential issues with such APIs.

thanks
— Hitesh 

On Aug 20, 2015, at 2:10 AM, Andre Kelpe <ak...@concurrentinc.com> wrote:

> Thanks for the answer. We have a work-around for now. I am going to make
> that inventory and submit a patch that changes the annotations from
> @Private to @LimitedPrivate.
> 
> From a semantic versioning point of view, I would still expect no breaking
> changes in a bug fix release, but if the Tez community at large can work
> with that, we have to accept that, I guess.
> 
> - André
> 
> On Tue, Aug 18, 2015 at 8:34 PM, Hitesh Shah <hi...@apache.org> wrote:
> 
>> Hello Andre,
>> 
>> For the most part, @Private is considered internal implementation and
>> subject to change at any point. In this case, even more so as it is an
>> *Impl class.
>> 
>> What we can do is try the following:
>>    - Look at all the various @Private classes being used by Cascading.
>>    - See which ones should not be used at all and which ones can be
>> considered to be a @LimitedPrivate for Cascading.
>>    - For LimitedPrivate apis, we would then try to be a bit more careful
>> with respect to changing/breaking these APIs. I would probably not say that
>> they have will have the same guarantees as @Public/@Stable but we can work
>> with the Cascading community to handle changes to these APIs in a workable
>> manner on an ongoing basis.
>> 
>> As for this API in question, for the short term fix, I guess a simple
>> approach might be to introduce a backward compatible API ( I believe one
>> which does not need the timeout param passed in )? Would you mind filing a
>> jira and hopefully provide a patch too?
>> 
>> thanks
>> — Hitesh
>> 
>> On Aug 18, 2015, at 8:01 AM, Andre Kelpe <ak...@concurrentinc.com> wrote:
>> 
>>> Hi,
>>> 
>>> I have found a small API incompatibility in the Tez 0.6.2 release. The
>>> DAGClientTimelineImpl constructor got a new parameter for time-outs. This
>>> was not present in 0.6.1 and from a semantic versioning point of view,
>> that
>>> is problematic. I know that the class is marked with the @Private
>>> annotation, but it would be great if such incompatible things aren't
>>> introduced in a bug-fix release. It would be easier for downsteam
>> projects,
>>> if you just added a second constructor.
>>> 
>>> Is that an oversight or are classes with the @Private annotation subject
>> to
>>> change and dowstream projects simply have to deal with it?
>>> 
>>> Thanks!
>>> 
>>> - André
>>> 
>>> --
>>> André Kelpe
>>> andre@concurrentinc.com
>>> http://concurrentinc.com
>> 
>> 
> 
> 
> -- 
> André Kelpe
> andre@concurrentinc.com
> http://concurrentinc.com


RE: api compatibility within a minor release

Posted by Bikas Saha <bi...@hortonworks.com>.
So what you would ideally like is a DAGClient.getVertexTaskStatus() (because internally DAGClient forks out to either the AM or the Timeline server behind the scenes so you don’t have to worry about it).

If so, please open a jira for that and we can put that on the roadmap.

-----Original Message-----
From: Chris K Wensel [mailto:chris@wensel.net] 
Sent: Thursday, August 20, 2015 12:02 PM
To: dev@tez.apache.org
Subject: Re: api compatibility within a minor release

hey guys

we recognize we are touching an internal api. 

and I think we are fine forcing our users to move to 0.6.2 due to the change in our minor release since 0.6.2 resolves issues our users and sub-projects need to be in production. actually, this is what we are doing in 3.0.2.

the api in question is the constructor in DAGClientTimelineImpl, no backward compatible constructor was provided. was a risk we took, and lost.

we sub-class DAGClientTimelineImpl in order to leverage existing code for fetching data from the timeline server instead of copying or re-implementing it — a reasonable practice.

we do this because there is no method on DAGClient for retrieving TaskStatus objects, but the data is in the timeline server. we require task level data in order to retain parity with the MapReduce implementation. 

once we moved on, we never filed an issue for the missing method (getVertexTaskStatus()).

in practice going to timeline server during runtime is a bit unsatisfactory since only started and completed events are stored in timeline server, so we cannot access intermediate counters as they progress. but going there eventually is required once the AM is shutdown and there are no status access (thus DAGClient falls back to timeline server, which we inherit by sub-classing, for the remaining methods).

ckw


> On Aug 20, 2015, at 11:29 AM, Bikas Saha <bi...@hortonworks.com> wrote:
> 
> It would be good to understand why that private method is being used. It was marked private because we did not want it to be used. So changing it to limited private kind of defeats the purpose, right?
> 
> If we can understand what the use case is then we can try to make it work without using private methods.
> 
> To be clear, this should not be considered a breaking change in the API since its not part of the documented API.
> 
> -----Original Message-----
> From: Andre Kelpe [mailto:akelpe@concurrentinc.com]
> Sent: Thursday, August 20, 2015 2:11 AM
> To: dev@tez.apache.org
> Subject: Re: api compatibility within a minor release
> 
> Thanks for the answer. We have a work-around for now. I am going to make that inventory and submit a patch that changes the annotations from @Private to @LimitedPrivate.
> 
> From a semantic versioning point of view, I would still expect no breaking changes in a bug fix release, but if the Tez community at large can work with that, we have to accept that, I guess.
> 
> - André
> 
> On Tue, Aug 18, 2015 at 8:34 PM, Hitesh Shah <hi...@apache.org> wrote:
> 
>> Hello Andre,
>> 
>> For the most part, @Private is considered internal implementation and 
>> subject to change at any point. In this case, even more so as it is 
>> an *Impl class.
>> 
>> What we can do is try the following:
>>    - Look at all the various @Private classes being used by Cascading.
>>    - See which ones should not be used at all and which ones can be 
>> considered to be a @LimitedPrivate for Cascading.
>>    - For LimitedPrivate apis, we would then try to be a bit more 
>> careful with respect to changing/breaking these APIs. I would 
>> probably not say that they have will have the same guarantees as 
>> @Public/@Stable but we can work with the Cascading community to 
>> handle changes to these APIs in a workable manner on an ongoing basis.
>> 
>> As for this API in question, for the short term fix, I guess a simple 
>> approach might be to introduce a backward compatible API ( I believe 
>> one which does not need the timeout param passed in )? Would you mind 
>> filing a jira and hopefully provide a patch too?
>> 
>> thanks
>> — Hitesh
>> 
>> On Aug 18, 2015, at 8:01 AM, Andre Kelpe <ak...@concurrentinc.com> wrote:
>> 
>>> Hi,
>>> 
>>> I have found a small API incompatibility in the Tez 0.6.2 release. 
>>> The DAGClientTimelineImpl constructor got a new parameter for 
>>> time-outs. This was not present in 0.6.1 and from a semantic 
>>> versioning point of view,
>> that
>>> is problematic. I know that the class is marked with the @Private 
>>> annotation, but it would be great if such incompatible things aren't 
>>> introduced in a bug-fix release. It would be easier for downsteam
>> projects,
>>> if you just added a second constructor.
>>> 
>>> Is that an oversight or are classes with the @Private annotation 
>>> subject
>> to
>>> change and dowstream projects simply have to deal with it?
>>> 
>>> Thanks!
>>> 
>>> - André
>>> 
>>> --
>>> André Kelpe
>>> andre@concurrentinc.com
>>> http://concurrentinc.com
>> 
>> 
> 
> 
> --
> André Kelpe
> andre@concurrentinc.com
> http://concurrentinc.com

—
Chris K Wensel
chris@wensel.net






Re: api compatibility within a minor release

Posted by Chris K Wensel <ch...@wensel.net>.
hey guys

we recognize we are touching an internal api. 

and I think we are fine forcing our users to move to 0.6.2 due to the change in our minor release since 0.6.2 resolves issues our users and sub-projects need to be in production. actually, this is what we are doing in 3.0.2.

the api in question is the constructor in DAGClientTimelineImpl, no backward compatible constructor was provided. was a risk we took, and lost.

we sub-class DAGClientTimelineImpl in order to leverage existing code for fetching data from the timeline server instead of copying or re-implementing it — a reasonable practice.

we do this because there is no method on DAGClient for retrieving TaskStatus objects, but the data is in the timeline server. we require task level data in order to retain parity with the MapReduce implementation. 

once we moved on, we never filed an issue for the missing method (getVertexTaskStatus()).

in practice going to timeline server during runtime is a bit unsatisfactory since only started and completed events are stored in timeline server, so we cannot access intermediate counters as they progress. but going there eventually is required once the AM is shutdown and there are no status access (thus DAGClient falls back to timeline server, which we inherit by sub-classing, for the remaining methods).

ckw


> On Aug 20, 2015, at 11:29 AM, Bikas Saha <bi...@hortonworks.com> wrote:
> 
> It would be good to understand why that private method is being used. It was marked private because we did not want it to be used. So changing it to limited private kind of defeats the purpose, right?
> 
> If we can understand what the use case is then we can try to make it work without using private methods.
> 
> To be clear, this should not be considered a breaking change in the API since its not part of the documented API.
> 
> -----Original Message-----
> From: Andre Kelpe [mailto:akelpe@concurrentinc.com] 
> Sent: Thursday, August 20, 2015 2:11 AM
> To: dev@tez.apache.org
> Subject: Re: api compatibility within a minor release
> 
> Thanks for the answer. We have a work-around for now. I am going to make that inventory and submit a patch that changes the annotations from @Private to @LimitedPrivate.
> 
> From a semantic versioning point of view, I would still expect no breaking changes in a bug fix release, but if the Tez community at large can work with that, we have to accept that, I guess.
> 
> - André
> 
> On Tue, Aug 18, 2015 at 8:34 PM, Hitesh Shah <hi...@apache.org> wrote:
> 
>> Hello Andre,
>> 
>> For the most part, @Private is considered internal implementation and 
>> subject to change at any point. In this case, even more so as it is an 
>> *Impl class.
>> 
>> What we can do is try the following:
>>    - Look at all the various @Private classes being used by Cascading.
>>    - See which ones should not be used at all and which ones can be 
>> considered to be a @LimitedPrivate for Cascading.
>>    - For LimitedPrivate apis, we would then try to be a bit more 
>> careful with respect to changing/breaking these APIs. I would probably 
>> not say that they have will have the same guarantees as 
>> @Public/@Stable but we can work with the Cascading community to handle 
>> changes to these APIs in a workable manner on an ongoing basis.
>> 
>> As for this API in question, for the short term fix, I guess a simple 
>> approach might be to introduce a backward compatible API ( I believe 
>> one which does not need the timeout param passed in )? Would you mind 
>> filing a jira and hopefully provide a patch too?
>> 
>> thanks
>> — Hitesh
>> 
>> On Aug 18, 2015, at 8:01 AM, Andre Kelpe <ak...@concurrentinc.com> wrote:
>> 
>>> Hi,
>>> 
>>> I have found a small API incompatibility in the Tez 0.6.2 release. 
>>> The DAGClientTimelineImpl constructor got a new parameter for 
>>> time-outs. This was not present in 0.6.1 and from a semantic 
>>> versioning point of view,
>> that
>>> is problematic. I know that the class is marked with the @Private 
>>> annotation, but it would be great if such incompatible things aren't 
>>> introduced in a bug-fix release. It would be easier for downsteam
>> projects,
>>> if you just added a second constructor.
>>> 
>>> Is that an oversight or are classes with the @Private annotation 
>>> subject
>> to
>>> change and dowstream projects simply have to deal with it?
>>> 
>>> Thanks!
>>> 
>>> - André
>>> 
>>> --
>>> André Kelpe
>>> andre@concurrentinc.com
>>> http://concurrentinc.com
>> 
>> 
> 
> 
> --
> André Kelpe
> andre@concurrentinc.com
> http://concurrentinc.com

—
Chris K Wensel
chris@wensel.net





RE: api compatibility within a minor release

Posted by Bikas Saha <bi...@hortonworks.com>.
It would be good to understand why that private method is being used. It was marked private because we did not want it to be used. So changing it to limited private kind of defeats the purpose, right?

If we can understand what the use case is then we can try to make it work without using private methods.

To be clear, this should not be considered a breaking change in the API since its not part of the documented API.

-----Original Message-----
From: Andre Kelpe [mailto:akelpe@concurrentinc.com] 
Sent: Thursday, August 20, 2015 2:11 AM
To: dev@tez.apache.org
Subject: Re: api compatibility within a minor release

Thanks for the answer. We have a work-around for now. I am going to make that inventory and submit a patch that changes the annotations from @Private to @LimitedPrivate.

From a semantic versioning point of view, I would still expect no breaking changes in a bug fix release, but if the Tez community at large can work with that, we have to accept that, I guess.

- André

On Tue, Aug 18, 2015 at 8:34 PM, Hitesh Shah <hi...@apache.org> wrote:

> Hello Andre,
>
> For the most part, @Private is considered internal implementation and 
> subject to change at any point. In this case, even more so as it is an 
> *Impl class.
>
> What we can do is try the following:
>     - Look at all the various @Private classes being used by Cascading.
>     - See which ones should not be used at all and which ones can be 
> considered to be a @LimitedPrivate for Cascading.
>     - For LimitedPrivate apis, we would then try to be a bit more 
> careful with respect to changing/breaking these APIs. I would probably 
> not say that they have will have the same guarantees as 
> @Public/@Stable but we can work with the Cascading community to handle 
> changes to these APIs in a workable manner on an ongoing basis.
>
> As for this API in question, for the short term fix, I guess a simple 
> approach might be to introduce a backward compatible API ( I believe 
> one which does not need the timeout param passed in )? Would you mind 
> filing a jira and hopefully provide a patch too?
>
> thanks
> — Hitesh
>
> On Aug 18, 2015, at 8:01 AM, Andre Kelpe <ak...@concurrentinc.com> wrote:
>
> > Hi,
> >
> > I have found a small API incompatibility in the Tez 0.6.2 release. 
> > The DAGClientTimelineImpl constructor got a new parameter for 
> > time-outs. This was not present in 0.6.1 and from a semantic 
> > versioning point of view,
> that
> > is problematic. I know that the class is marked with the @Private 
> > annotation, but it would be great if such incompatible things aren't 
> > introduced in a bug-fix release. It would be easier for downsteam
> projects,
> > if you just added a second constructor.
> >
> > Is that an oversight or are classes with the @Private annotation 
> > subject
> to
> > change and dowstream projects simply have to deal with it?
> >
> > Thanks!
> >
> > - André
> >
> > --
> > André Kelpe
> > andre@concurrentinc.com
> > http://concurrentinc.com
>
>


--
André Kelpe
andre@concurrentinc.com
http://concurrentinc.com

Re: api compatibility within a minor release

Posted by Andre Kelpe <ak...@concurrentinc.com>.
Thanks for the answer. We have a work-around for now. I am going to make
that inventory and submit a patch that changes the annotations from
@Private to @LimitedPrivate.

>From a semantic versioning point of view, I would still expect no breaking
changes in a bug fix release, but if the Tez community at large can work
with that, we have to accept that, I guess.

- André

On Tue, Aug 18, 2015 at 8:34 PM, Hitesh Shah <hi...@apache.org> wrote:

> Hello Andre,
>
> For the most part, @Private is considered internal implementation and
> subject to change at any point. In this case, even more so as it is an
> *Impl class.
>
> What we can do is try the following:
>     - Look at all the various @Private classes being used by Cascading.
>     - See which ones should not be used at all and which ones can be
> considered to be a @LimitedPrivate for Cascading.
>     - For LimitedPrivate apis, we would then try to be a bit more careful
> with respect to changing/breaking these APIs. I would probably not say that
> they have will have the same guarantees as @Public/@Stable but we can work
> with the Cascading community to handle changes to these APIs in a workable
> manner on an ongoing basis.
>
> As for this API in question, for the short term fix, I guess a simple
> approach might be to introduce a backward compatible API ( I believe one
> which does not need the timeout param passed in )? Would you mind filing a
> jira and hopefully provide a patch too?
>
> thanks
> — Hitesh
>
> On Aug 18, 2015, at 8:01 AM, Andre Kelpe <ak...@concurrentinc.com> wrote:
>
> > Hi,
> >
> > I have found a small API incompatibility in the Tez 0.6.2 release. The
> > DAGClientTimelineImpl constructor got a new parameter for time-outs. This
> > was not present in 0.6.1 and from a semantic versioning point of view,
> that
> > is problematic. I know that the class is marked with the @Private
> > annotation, but it would be great if such incompatible things aren't
> > introduced in a bug-fix release. It would be easier for downsteam
> projects,
> > if you just added a second constructor.
> >
> > Is that an oversight or are classes with the @Private annotation subject
> to
> > change and dowstream projects simply have to deal with it?
> >
> > Thanks!
> >
> > - André
> >
> > --
> > André Kelpe
> > andre@concurrentinc.com
> > http://concurrentinc.com
>
>


-- 
André Kelpe
andre@concurrentinc.com
http://concurrentinc.com

Re: api compatibility within a minor release

Posted by Hitesh Shah <hi...@apache.org>.
Hello Andre, 

For the most part, @Private is considered internal implementation and subject to change at any point. In this case, even more so as it is an *Impl class. 

What we can do is try the following: 
    - Look at all the various @Private classes being used by Cascading.
    - See which ones should not be used at all and which ones can be considered to be a @LimitedPrivate for Cascading.
    - For LimitedPrivate apis, we would then try to be a bit more careful with respect to changing/breaking these APIs. I would probably not say that they have will have the same guarantees as @Public/@Stable but we can work with the Cascading community to handle changes to these APIs in a workable manner on an ongoing basis. 

As for this API in question, for the short term fix, I guess a simple approach might be to introduce a backward compatible API ( I believe one which does not need the timeout param passed in )? Would you mind filing a jira and hopefully provide a patch too?

thanks
— Hitesh

On Aug 18, 2015, at 8:01 AM, Andre Kelpe <ak...@concurrentinc.com> wrote:

> Hi,
> 
> I have found a small API incompatibility in the Tez 0.6.2 release. The
> DAGClientTimelineImpl constructor got a new parameter for time-outs. This
> was not present in 0.6.1 and from a semantic versioning point of view, that
> is problematic. I know that the class is marked with the @Private
> annotation, but it would be great if such incompatible things aren't
> introduced in a bug-fix release. It would be easier for downsteam projects,
> if you just added a second constructor.
> 
> Is that an oversight or are classes with the @Private annotation subject to
> change and dowstream projects simply have to deal with it?
> 
> Thanks!
> 
> - André
> 
> -- 
> André Kelpe
> andre@concurrentinc.com
> http://concurrentinc.com