Re: [DISCUSS] Using Guava in Flink "core" packages

[Thomas Weise]

Hi,

Thanks for the follow-up.

@Aljoscha Krettek  if there is consensus, can we also
backport FLINK-11501 for 1.8.1? (The Guava dependency was the reason we
didn't.)

Thanks,
Thomas


On Mon, Mar 25, 2019 at 8:07 AM Stephan Ewen  wrote:

> I would be fine with that as well.
>
> Let's keep the attitude to maintain a slim dependency footprint, but Guava
> could be part of that small footprint.
> As long as we ensure it is our vendored (pre-shaded) version and all
> modules use the same version (managed version in the root pom).
>
> Best,
> Stephan
>
>
> On Mon, Mar 25, 2019 at 10:34 AM Aljoscha Krettek 
> wrote:
>
> > Hi,
> >
> > By now, I think it’s fine to have Guava (our shaded guava) as a
> dependency
> > in flink-core. It’s a very useful library and I don’t see problems with
> > version clashes because it’s relocated to our namespace.
> >
> > If no-one objects, I would just leave the dependency in as-is and
> conclude
> > this discussion.
> >
> > Best,
> > Aljoscha
> >
> > > On 13. Mar 2019, at 05:10, Thomas Weise  wrote:
> > >
> > > I want to bring this back to attention - with the concern raised here
> we
> > > are still looking for a potentially better answer to the original issue
> > [1].
> > >
> > > We have a class that depends on Guava (in its implementation, not the
> > > public interface) and we want to make it available for use with
> multiple
> > > connectors.
> > >
> > > Thanks,
> > > Thomas
> > >
> > > [1]
> > >
> >
> https://lists.apache.org/thread.html/3de9d2353cf22aea0448fb744314103b5f88195216acc3bff449354a@%3Cdev.flink.apache.org%3E
> > >
> > > On Wed, Mar 6, 2019 at 7:13 AM Thomas Weise  wrote:
> > >
> > >> How I managed to do that..
> > >>
> > >> Here is the discussion about the shared package:
> > >>
> > >>
> > >>
> >
> https://lists.apache.org/thread.html/3de9d2353cf22aea0448fb744314103b5f88195216acc3bff449354a@%3Cdev.flink.apache.org%3E
> > >>
> > >>
> > >> On Wed, Mar 6, 2019 at 7:10 AM Aljoscha Krettek 
> > >> wrote:
> > >>
> > >>> I think the two links are identical.
> > >>>
> >  On 6. Mar 2019, at 16:05, Thomas Weise  wrote:
> > 
> >  For more context, see [1] [2]
> > 
> >  The GuavaFlinkConnectorRateLimiter is an implementation of the rate
> > >>> limiter
> >  interface that uses Guava. It is not a test class, it is intended to
> > be
> >  used in applications and the (shaded) Guava isn't user facing.
> > 
> >  [1]
> > 
> > >>>
> >
> https://lists.apache.org/thread.html/3599d95020604e2476bd794c55a11bc1c3a958a83a3c9c45c50630c1@%3Cdev.flink.apache.org%3E
> >  [2]
> > 
> > >>>
> >
> https://lists.apache.org/thread.html/3599d95020604e2476bd794c55a11bc1c3a958a83a3c9c45c50630c1@%3Cdev.flink.apache.org%3E
> > 
> > 
> >  On Wed, Mar 6, 2019 at 6:41 AM Chesnay Schepler  >
> > >>> wrote:
> > 
> > > I fully agree that we should write down this kind of conventions
> that
> > > committers have setup implicitly.
> > >
> > > I agree that we should keep flink-core as lean as possible.
> > >
> > > On guava usages in general my current stance is that we should only
> > use
> > > guava if necessary. Spreading it (or other dependencies for that
> > >>> matter)
> > > to far just creates headaches when bumping the dependency.
> > >
> > > One core principle that we *must* follow is that shaded
> dependencies
> > >>> are
> > > neither exposed to user-code nor contained in the user-jar.
> Otherwise
> > >>> we
> > > end up again in a situation where we cannot increment dependencies
> > > without breaking compatibility for users.
> > >
> > > The PR at hand has a few issues in this regard. The guava limiter
> is
> > > only used in tests but is not a test class, yet isn't annotated
> with
> > > either Public(Evolving)/Internal. As it stands I cannot judge
> whether
> > > this is truly supposed to be an example / test utility or actually
> a
> > > user-facing class.
> > > If this is to be used by connectors, which are included in the
> > >>> user-jar,
> > > then we're violating the principle above, in which case the class
> > >>> should
> > > be relocated/removed.
> > >
> > > On 06.03.2019 15:10, Aljoscha Krettek wrote:
> > >> Hi,
> > >>
> > >> I recently saw that we added a dependency on our shaded-guava to
> > > flink-core [1]. Just for the record, I don’t want do diminish the
> > > contributions of anyone involved in the PR in any way. It just made
> > me
> > > realise that we have some implicit agreements or assumptions about
> > >>> adding
> > > certain things to core packages that we might never have really
> > >>> discussed.
> > > I think we should do that now.
> > >>
> > >> Quite some time ago an effort was started to reduce our dependency
> > on
> > > Guava [2] because of some problems with version stability and
> > >>> dependency
> > > conflicts. At some later point, we created shaded guava 

Re: [DISCUSS] Using Guava in Flink "core" packages

[Stephan Ewen]

I would be fine with that as well.

Let's keep the attitude to maintain a slim dependency footprint, but Guava
could be part of that small footprint.
As long as we ensure it is our vendored (pre-shaded) version and all
modules use the same version (managed version in the root pom).

Best,
Stephan


On Mon, Mar 25, 2019 at 10:34 AM Aljoscha Krettek 
wrote:

> Hi,
>
> By now, I think it’s fine to have Guava (our shaded guava) as a dependency
> in flink-core. It’s a very useful library and I don’t see problems with
> version clashes because it’s relocated to our namespace.
>
> If no-one objects, I would just leave the dependency in as-is and conclude
> this discussion.
>
> Best,
> Aljoscha
>
> > On 13. Mar 2019, at 05:10, Thomas Weise  wrote:
> >
> > I want to bring this back to attention - with the concern raised here we
> > are still looking for a potentially better answer to the original issue
> [1].
> >
> > We have a class that depends on Guava (in its implementation, not the
> > public interface) and we want to make it available for use with multiple
> > connectors.
> >
> > Thanks,
> > Thomas
> >
> > [1]
> >
> https://lists.apache.org/thread.html/3de9d2353cf22aea0448fb744314103b5f88195216acc3bff449354a@%3Cdev.flink.apache.org%3E
> >
> > On Wed, Mar 6, 2019 at 7:13 AM Thomas Weise  wrote:
> >
> >> How I managed to do that..
> >>
> >> Here is the discussion about the shared package:
> >>
> >>
> >>
> https://lists.apache.org/thread.html/3de9d2353cf22aea0448fb744314103b5f88195216acc3bff449354a@%3Cdev.flink.apache.org%3E
> >>
> >>
> >> On Wed, Mar 6, 2019 at 7:10 AM Aljoscha Krettek 
> >> wrote:
> >>
> >>> I think the two links are identical.
> >>>
>  On 6. Mar 2019, at 16:05, Thomas Weise  wrote:
> 
>  For more context, see [1] [2]
> 
>  The GuavaFlinkConnectorRateLimiter is an implementation of the rate
> >>> limiter
>  interface that uses Guava. It is not a test class, it is intended to
> be
>  used in applications and the (shaded) Guava isn't user facing.
> 
>  [1]
> 
> >>>
> https://lists.apache.org/thread.html/3599d95020604e2476bd794c55a11bc1c3a958a83a3c9c45c50630c1@%3Cdev.flink.apache.org%3E
>  [2]
> 
> >>>
> https://lists.apache.org/thread.html/3599d95020604e2476bd794c55a11bc1c3a958a83a3c9c45c50630c1@%3Cdev.flink.apache.org%3E
> 
> 
>  On Wed, Mar 6, 2019 at 6:41 AM Chesnay Schepler 
> >>> wrote:
> 
> > I fully agree that we should write down this kind of conventions that
> > committers have setup implicitly.
> >
> > I agree that we should keep flink-core as lean as possible.
> >
> > On guava usages in general my current stance is that we should only
> use
> > guava if necessary. Spreading it (or other dependencies for that
> >>> matter)
> > to far just creates headaches when bumping the dependency.
> >
> > One core principle that we *must* follow is that shaded dependencies
> >>> are
> > neither exposed to user-code nor contained in the user-jar. Otherwise
> >>> we
> > end up again in a situation where we cannot increment dependencies
> > without breaking compatibility for users.
> >
> > The PR at hand has a few issues in this regard. The guava limiter is
> > only used in tests but is not a test class, yet isn't annotated with
> > either Public(Evolving)/Internal. As it stands I cannot judge whether
> > this is truly supposed to be an example / test utility or actually a
> > user-facing class.
> > If this is to be used by connectors, which are included in the
> >>> user-jar,
> > then we're violating the principle above, in which case the class
> >>> should
> > be relocated/removed.
> >
> > On 06.03.2019 15:10, Aljoscha Krettek wrote:
> >> Hi,
> >>
> >> I recently saw that we added a dependency on our shaded-guava to
> > flink-core [1]. Just for the record, I don’t want do diminish the
> > contributions of anyone involved in the PR in any way. It just made
> me
> > realise that we have some implicit agreements or assumptions about
> >>> adding
> > certain things to core packages that we might never have really
> >>> discussed.
> > I think we should do that now.
> >>
> >> Quite some time ago an effort was started to reduce our dependency
> on
> > Guava [2] because of some problems with version stability and
> >>> dependency
> > conflicts. At some later point, we created shaded guava so that we
> >>> could
> > use it without clashes [3]. I believe we now have shaded Guava only
> in
> > runtime modules where it wasn’t easy to remove and CEP.
> >>
> >> With the creation of shaded guava, I think we are in a bit of a
> limbo
> > situation where it is not exactly clear what our stance towards it
> is,
> > because it is easy to add to modules, as evident by the
> aforementioned
> >>> PR.
> > I think we should discuss that situation and agree upon a common
> >>> stance on
> 

Re: [DISCUSS] Using Guava in Flink "core" packages

[Aljoscha Krettek]

Hi,

By now, I think it’s fine to have Guava (our shaded guava) as a dependency in 
flink-core. It’s a very useful library and I don’t see problems with version 
clashes because it’s relocated to our namespace.

If no-one objects, I would just leave the dependency in as-is and conclude this 
discussion.

Best,
Aljoscha

> On 13. Mar 2019, at 05:10, Thomas Weise  wrote:
> 
> I want to bring this back to attention - with the concern raised here we
> are still looking for a potentially better answer to the original issue [1].
> 
> We have a class that depends on Guava (in its implementation, not the
> public interface) and we want to make it available for use with multiple
> connectors.
> 
> Thanks,
> Thomas
> 
> [1]
> https://lists.apache.org/thread.html/3de9d2353cf22aea0448fb744314103b5f88195216acc3bff449354a@%3Cdev.flink.apache.org%3E
> 
> On Wed, Mar 6, 2019 at 7:13 AM Thomas Weise  wrote:
> 
>> How I managed to do that..
>> 
>> Here is the discussion about the shared package:
>> 
>> 
>> https://lists.apache.org/thread.html/3de9d2353cf22aea0448fb744314103b5f88195216acc3bff449354a@%3Cdev.flink.apache.org%3E
>> 
>> 
>> On Wed, Mar 6, 2019 at 7:10 AM Aljoscha Krettek 
>> wrote:
>> 
>>> I think the two links are identical.
>>> 
 On 6. Mar 2019, at 16:05, Thomas Weise  wrote:
 
 For more context, see [1] [2]
 
 The GuavaFlinkConnectorRateLimiter is an implementation of the rate
>>> limiter
 interface that uses Guava. It is not a test class, it is intended to be
 used in applications and the (shaded) Guava isn't user facing.
 
 [1]
 
>>> https://lists.apache.org/thread.html/3599d95020604e2476bd794c55a11bc1c3a958a83a3c9c45c50630c1@%3Cdev.flink.apache.org%3E
 [2]
 
>>> https://lists.apache.org/thread.html/3599d95020604e2476bd794c55a11bc1c3a958a83a3c9c45c50630c1@%3Cdev.flink.apache.org%3E
 
 
 On Wed, Mar 6, 2019 at 6:41 AM Chesnay Schepler 
>>> wrote:
 
> I fully agree that we should write down this kind of conventions that
> committers have setup implicitly.
> 
> I agree that we should keep flink-core as lean as possible.
> 
> On guava usages in general my current stance is that we should only use
> guava if necessary. Spreading it (or other dependencies for that
>>> matter)
> to far just creates headaches when bumping the dependency.
> 
> One core principle that we *must* follow is that shaded dependencies
>>> are
> neither exposed to user-code nor contained in the user-jar. Otherwise
>>> we
> end up again in a situation where we cannot increment dependencies
> without breaking compatibility for users.
> 
> The PR at hand has a few issues in this regard. The guava limiter is
> only used in tests but is not a test class, yet isn't annotated with
> either Public(Evolving)/Internal. As it stands I cannot judge whether
> this is truly supposed to be an example / test utility or actually a
> user-facing class.
> If this is to be used by connectors, which are included in the
>>> user-jar,
> then we're violating the principle above, in which case the class
>>> should
> be relocated/removed.
> 
> On 06.03.2019 15:10, Aljoscha Krettek wrote:
>> Hi,
>> 
>> I recently saw that we added a dependency on our shaded-guava to
> flink-core [1]. Just for the record, I don’t want do diminish the
> contributions of anyone involved in the PR in any way. It just made me
> realise that we have some implicit agreements or assumptions about
>>> adding
> certain things to core packages that we might never have really
>>> discussed.
> I think we should do that now.
>> 
>> Quite some time ago an effort was started to reduce our dependency on
> Guava [2] because of some problems with version stability and
>>> dependency
> conflicts. At some later point, we created shaded guava so that we
>>> could
> use it without clashes [3]. I believe we now have shaded Guava only in
> runtime modules where it wasn’t easy to remove and CEP.
>> 
>> With the creation of shaded guava, I think we are in a bit of a limbo
> situation where it is not exactly clear what our stance towards it is,
> because it is easy to add to modules, as evident by the aforementioned
>>> PR.
> I think we should discuss that situation and agree upon a common
>>> stance on
> the topic.
>> 
>> In general, I think the surface (which includes classes, interfaces,
>>> and
> dependencies, among other things) of core modules should be kept as
>>> lean as
> possible.  (all modules really)
>> 
>> What do you think?
>> 
>> Best,
>> Aljoscha
>> 
>> [1] https://github.com/apache/flink/pull/7679 <
> https://github.com/apache/flink/pull/7679>
>> [2] https://issues.apache.org/jira/browse/FLINK-3700 <
> https://issues.apache.org/jira/browse/FLINK-3700>
>> [3] 

Re: [DISCUSS] Using Guava in Flink "core" packages

[Thomas Weise]

I want to bring this back to attention - with the concern raised here we
are still looking for a potentially better answer to the original issue [1].

We have a class that depends on Guava (in its implementation, not the
public interface) and we want to make it available for use with multiple
connectors.

Thanks,
Thomas

[1]
https://lists.apache.org/thread.html/3de9d2353cf22aea0448fb744314103b5f88195216acc3bff449354a@%3Cdev.flink.apache.org%3E

On Wed, Mar 6, 2019 at 7:13 AM Thomas Weise  wrote:

> How I managed to do that..
>
> Here is the discussion about the shared package:
>
>
> https://lists.apache.org/thread.html/3de9d2353cf22aea0448fb744314103b5f88195216acc3bff449354a@%3Cdev.flink.apache.org%3E
>
>
> On Wed, Mar 6, 2019 at 7:10 AM Aljoscha Krettek 
> wrote:
>
>> I think the two links are identical.
>>
>> > On 6. Mar 2019, at 16:05, Thomas Weise  wrote:
>> >
>> > For more context, see [1] [2]
>> >
>> > The GuavaFlinkConnectorRateLimiter is an implementation of the rate
>> limiter
>> > interface that uses Guava. It is not a test class, it is intended to be
>> > used in applications and the (shaded) Guava isn't user facing.
>> >
>> > [1]
>> >
>> https://lists.apache.org/thread.html/3599d95020604e2476bd794c55a11bc1c3a958a83a3c9c45c50630c1@%3Cdev.flink.apache.org%3E
>> > [2]
>> >
>> https://lists.apache.org/thread.html/3599d95020604e2476bd794c55a11bc1c3a958a83a3c9c45c50630c1@%3Cdev.flink.apache.org%3E
>> >
>> >
>> > On Wed, Mar 6, 2019 at 6:41 AM Chesnay Schepler 
>> wrote:
>> >
>> >> I fully agree that we should write down this kind of conventions that
>> >> committers have setup implicitly.
>> >>
>> >> I agree that we should keep flink-core as lean as possible.
>> >>
>> >> On guava usages in general my current stance is that we should only use
>> >> guava if necessary. Spreading it (or other dependencies for that
>> matter)
>> >> to far just creates headaches when bumping the dependency.
>> >>
>> >> One core principle that we *must* follow is that shaded dependencies
>> are
>> >> neither exposed to user-code nor contained in the user-jar. Otherwise
>> we
>> >> end up again in a situation where we cannot increment dependencies
>> >> without breaking compatibility for users.
>> >>
>> >> The PR at hand has a few issues in this regard. The guava limiter is
>> >> only used in tests but is not a test class, yet isn't annotated with
>> >> either Public(Evolving)/Internal. As it stands I cannot judge whether
>> >> this is truly supposed to be an example / test utility or actually a
>> >> user-facing class.
>> >> If this is to be used by connectors, which are included in the
>> user-jar,
>> >> then we're violating the principle above, in which case the class
>> should
>> >> be relocated/removed.
>> >>
>> >> On 06.03.2019 15:10, Aljoscha Krettek wrote:
>> >>> Hi,
>> >>>
>> >>> I recently saw that we added a dependency on our shaded-guava to
>> >> flink-core [1]. Just for the record, I don’t want do diminish the
>> >> contributions of anyone involved in the PR in any way. It just made me
>> >> realise that we have some implicit agreements or assumptions about
>> adding
>> >> certain things to core packages that we might never have really
>> discussed.
>> >> I think we should do that now.
>> >>>
>> >>> Quite some time ago an effort was started to reduce our dependency on
>> >> Guava [2] because of some problems with version stability and
>> dependency
>> >> conflicts. At some later point, we created shaded guava so that we
>> could
>> >> use it without clashes [3]. I believe we now have shaded Guava only in
>> >> runtime modules where it wasn’t easy to remove and CEP.
>> >>>
>> >>> With the creation of shaded guava, I think we are in a bit of a limbo
>> >> situation where it is not exactly clear what our stance towards it is,
>> >> because it is easy to add to modules, as evident by the aforementioned
>> PR.
>> >> I think we should discuss that situation and agree upon a common
>> stance on
>> >> the topic.
>> >>>
>> >>> In general, I think the surface (which includes classes, interfaces,
>> and
>> >> dependencies, among other things) of core modules should be kept as
>> lean as
>> >> possible.  (all modules really)
>> >>>
>> >>> What do you think?
>> >>>
>> >>> Best,
>> >>> Aljoscha
>> >>>
>> >>> [1] https://github.com/apache/flink/pull/7679 <
>> >> https://github.com/apache/flink/pull/7679>
>> >>> [2] https://issues.apache.org/jira/browse/FLINK-3700 <
>> >> https://issues.apache.org/jira/browse/FLINK-3700>
>> >>> [3] https://issues.apache.org/jira/browse/FLINK-6982
>> >>
>> >>
>> >>
>>
>>

Re: [DISCUSS] Using Guava in Flink "core" packages

[Thomas Weise]

How I managed to do that..

Here is the discussion about the shared package:

https://lists.apache.org/thread.html/3de9d2353cf22aea0448fb744314103b5f88195216acc3bff449354a@%3Cdev.flink.apache.org%3E


On Wed, Mar 6, 2019 at 7:10 AM Aljoscha Krettek  wrote:

> I think the two links are identical.
>
> > On 6. Mar 2019, at 16:05, Thomas Weise  wrote:
> >
> > For more context, see [1] [2]
> >
> > The GuavaFlinkConnectorRateLimiter is an implementation of the rate
> limiter
> > interface that uses Guava. It is not a test class, it is intended to be
> > used in applications and the (shaded) Guava isn't user facing.
> >
> > [1]
> >
> https://lists.apache.org/thread.html/3599d95020604e2476bd794c55a11bc1c3a958a83a3c9c45c50630c1@%3Cdev.flink.apache.org%3E
> > [2]
> >
> https://lists.apache.org/thread.html/3599d95020604e2476bd794c55a11bc1c3a958a83a3c9c45c50630c1@%3Cdev.flink.apache.org%3E
> >
> >
> > On Wed, Mar 6, 2019 at 6:41 AM Chesnay Schepler 
> wrote:
> >
> >> I fully agree that we should write down this kind of conventions that
> >> committers have setup implicitly.
> >>
> >> I agree that we should keep flink-core as lean as possible.
> >>
> >> On guava usages in general my current stance is that we should only use
> >> guava if necessary. Spreading it (or other dependencies for that matter)
> >> to far just creates headaches when bumping the dependency.
> >>
> >> One core principle that we *must* follow is that shaded dependencies are
> >> neither exposed to user-code nor contained in the user-jar. Otherwise we
> >> end up again in a situation where we cannot increment dependencies
> >> without breaking compatibility for users.
> >>
> >> The PR at hand has a few issues in this regard. The guava limiter is
> >> only used in tests but is not a test class, yet isn't annotated with
> >> either Public(Evolving)/Internal. As it stands I cannot judge whether
> >> this is truly supposed to be an example / test utility or actually a
> >> user-facing class.
> >> If this is to be used by connectors, which are included in the user-jar,
> >> then we're violating the principle above, in which case the class should
> >> be relocated/removed.
> >>
> >> On 06.03.2019 15:10, Aljoscha Krettek wrote:
> >>> Hi,
> >>>
> >>> I recently saw that we added a dependency on our shaded-guava to
> >> flink-core [1]. Just for the record, I don’t want do diminish the
> >> contributions of anyone involved in the PR in any way. It just made me
> >> realise that we have some implicit agreements or assumptions about
> adding
> >> certain things to core packages that we might never have really
> discussed.
> >> I think we should do that now.
> >>>
> >>> Quite some time ago an effort was started to reduce our dependency on
> >> Guava [2] because of some problems with version stability and dependency
> >> conflicts. At some later point, we created shaded guava so that we could
> >> use it without clashes [3]. I believe we now have shaded Guava only in
> >> runtime modules where it wasn’t easy to remove and CEP.
> >>>
> >>> With the creation of shaded guava, I think we are in a bit of a limbo
> >> situation where it is not exactly clear what our stance towards it is,
> >> because it is easy to add to modules, as evident by the aforementioned
> PR.
> >> I think we should discuss that situation and agree upon a common stance
> on
> >> the topic.
> >>>
> >>> In general, I think the surface (which includes classes, interfaces,
> and
> >> dependencies, among other things) of core modules should be kept as
> lean as
> >> possible.  (all modules really)
> >>>
> >>> What do you think?
> >>>
> >>> Best,
> >>> Aljoscha
> >>>
> >>> [1] https://github.com/apache/flink/pull/7679 <
> >> https://github.com/apache/flink/pull/7679>
> >>> [2] https://issues.apache.org/jira/browse/FLINK-3700 <
> >> https://issues.apache.org/jira/browse/FLINK-3700>
> >>> [3] https://issues.apache.org/jira/browse/FLINK-6982
> >>
> >>
> >>
>
>

Re: [DISCUSS] Using Guava in Flink "core" packages

[Aljoscha Krettek]

I think the two links are identical.

> On 6. Mar 2019, at 16:05, Thomas Weise  wrote:
> 
> For more context, see [1] [2]
> 
> The GuavaFlinkConnectorRateLimiter is an implementation of the rate limiter
> interface that uses Guava. It is not a test class, it is intended to be
> used in applications and the (shaded) Guava isn't user facing.
> 
> [1]
> https://lists.apache.org/thread.html/3599d95020604e2476bd794c55a11bc1c3a958a83a3c9c45c50630c1@%3Cdev.flink.apache.org%3E
> [2]
> https://lists.apache.org/thread.html/3599d95020604e2476bd794c55a11bc1c3a958a83a3c9c45c50630c1@%3Cdev.flink.apache.org%3E
> 
> 
> On Wed, Mar 6, 2019 at 6:41 AM Chesnay Schepler  wrote:
> 
>> I fully agree that we should write down this kind of conventions that
>> committers have setup implicitly.
>> 
>> I agree that we should keep flink-core as lean as possible.
>> 
>> On guava usages in general my current stance is that we should only use
>> guava if necessary. Spreading it (or other dependencies for that matter)
>> to far just creates headaches when bumping the dependency.
>> 
>> One core principle that we *must* follow is that shaded dependencies are
>> neither exposed to user-code nor contained in the user-jar. Otherwise we
>> end up again in a situation where we cannot increment dependencies
>> without breaking compatibility for users.
>> 
>> The PR at hand has a few issues in this regard. The guava limiter is
>> only used in tests but is not a test class, yet isn't annotated with
>> either Public(Evolving)/Internal. As it stands I cannot judge whether
>> this is truly supposed to be an example / test utility or actually a
>> user-facing class.
>> If this is to be used by connectors, which are included in the user-jar,
>> then we're violating the principle above, in which case the class should
>> be relocated/removed.
>> 
>> On 06.03.2019 15:10, Aljoscha Krettek wrote:
>>> Hi,
>>> 
>>> I recently saw that we added a dependency on our shaded-guava to
>> flink-core [1]. Just for the record, I don’t want do diminish the
>> contributions of anyone involved in the PR in any way. It just made me
>> realise that we have some implicit agreements or assumptions about adding
>> certain things to core packages that we might never have really discussed.
>> I think we should do that now.
>>> 
>>> Quite some time ago an effort was started to reduce our dependency on
>> Guava [2] because of some problems with version stability and dependency
>> conflicts. At some later point, we created shaded guava so that we could
>> use it without clashes [3]. I believe we now have shaded Guava only in
>> runtime modules where it wasn’t easy to remove and CEP.
>>> 
>>> With the creation of shaded guava, I think we are in a bit of a limbo
>> situation where it is not exactly clear what our stance towards it is,
>> because it is easy to add to modules, as evident by the aforementioned PR.
>> I think we should discuss that situation and agree upon a common stance on
>> the topic.
>>> 
>>> In general, I think the surface (which includes classes, interfaces, and
>> dependencies, among other things) of core modules should be kept as lean as
>> possible.  (all modules really)
>>> 
>>> What do you think?
>>> 
>>> Best,
>>> Aljoscha
>>> 
>>> [1] https://github.com/apache/flink/pull/7679 <
>> https://github.com/apache/flink/pull/7679>
>>> [2] https://issues.apache.org/jira/browse/FLINK-3700 <
>> https://issues.apache.org/jira/browse/FLINK-3700>
>>> [3] https://issues.apache.org/jira/browse/FLINK-6982
>> 
>> 
>> 

Re: [DISCUSS] Using Guava in Flink "core" packages

[Thomas Weise]

For more context, see [1] [2]

The GuavaFlinkConnectorRateLimiter is an implementation of the rate limiter
interface that uses Guava. It is not a test class, it is intended to be
used in applications and the (shaded) Guava isn't user facing.

[1]
https://lists.apache.org/thread.html/3599d95020604e2476bd794c55a11bc1c3a958a83a3c9c45c50630c1@%3Cdev.flink.apache.org%3E
[2]
https://lists.apache.org/thread.html/3599d95020604e2476bd794c55a11bc1c3a958a83a3c9c45c50630c1@%3Cdev.flink.apache.org%3E


On Wed, Mar 6, 2019 at 6:41 AM Chesnay Schepler  wrote:

> I fully agree that we should write down this kind of conventions that
> committers have setup implicitly.
>
> I agree that we should keep flink-core as lean as possible.
>
> On guava usages in general my current stance is that we should only use
> guava if necessary. Spreading it (or other dependencies for that matter)
> to far just creates headaches when bumping the dependency.
>
> One core principle that we *must* follow is that shaded dependencies are
> neither exposed to user-code nor contained in the user-jar. Otherwise we
> end up again in a situation where we cannot increment dependencies
> without breaking compatibility for users.
>
> The PR at hand has a few issues in this regard. The guava limiter is
> only used in tests but is not a test class, yet isn't annotated with
> either Public(Evolving)/Internal. As it stands I cannot judge whether
> this is truly supposed to be an example / test utility or actually a
> user-facing class.
> If this is to be used by connectors, which are included in the user-jar,
> then we're violating the principle above, in which case the class should
> be relocated/removed.
>
> On 06.03.2019 15:10, Aljoscha Krettek wrote:
> > Hi,
> >
> > I recently saw that we added a dependency on our shaded-guava to
> flink-core [1]. Just for the record, I don’t want do diminish the
> contributions of anyone involved in the PR in any way. It just made me
> realise that we have some implicit agreements or assumptions about adding
> certain things to core packages that we might never have really discussed.
> I think we should do that now.
> >
> > Quite some time ago an effort was started to reduce our dependency on
> Guava [2] because of some problems with version stability and dependency
> conflicts. At some later point, we created shaded guava so that we could
> use it without clashes [3]. I believe we now have shaded Guava only in
> runtime modules where it wasn’t easy to remove and CEP.
> >
> > With the creation of shaded guava, I think we are in a bit of a limbo
> situation where it is not exactly clear what our stance towards it is,
> because it is easy to add to modules, as evident by the aforementioned PR.
> I think we should discuss that situation and agree upon a common stance on
> the topic.
> >
> > In general, I think the surface (which includes classes, interfaces, and
> dependencies, among other things) of core modules should be kept as lean as
> possible.  (all modules really)
> >
> > What do you think?
> >
> > Best,
> > Aljoscha
> >
> > [1] https://github.com/apache/flink/pull/7679 <
> https://github.com/apache/flink/pull/7679>
> > [2] https://issues.apache.org/jira/browse/FLINK-3700 <
> https://issues.apache.org/jira/browse/FLINK-3700>
> > [3] https://issues.apache.org/jira/browse/FLINK-6982
>
>
>

Re: [DISCUSS] Using Guava in Flink "core" packages

[Timo Walther]

Hi,

yes I also fully agree that it is time to write down all these implicit 
convensions that we've learned throught the last years. The Flink 
community is growing quite rapidly right now and we must ensure that the 
same mistakes do not repeat again.


Keeping the number of dependencies low is a very good example. Even if 
Guava is shaded, some libraries also depend on Guava and maybe only 
support a certain set of versions. Calcite is a good example. They use 
Guava heavily and must even note which Guava versions they currently 
maintain in the release notes [1] (currently 19-27). In most cases there 
are core Java alternatives or one method must simply be added to some 
utility class instead.


Stephan, me and other people work on code quality guide. We will publish 
this soon to the ML for discussion.


Regards,
Timo

[1] https://calcite.apache.org/docs/history.html#v1-18-0

Am 06.03.19 um 15:40 schrieb Chesnay Schepler:
I fully agree that we should write down this kind of conventions that 
committers have setup implicitly.


I agree that we should keep flink-core as lean as possible.

On guava usages in general my current stance is that we should only 
use guava if necessary. Spreading it (or other dependencies for that 
matter) to far just creates headaches when bumping the dependency.


One core principle that we *must* follow is that shaded dependencies 
are neither exposed to user-code nor contained in the user-jar. 
Otherwise we end up again in a situation where we cannot increment 
dependencies without breaking compatibility for users.


The PR at hand has a few issues in this regard. The guava limiter is 
only used in tests but is not a test class, yet isn't annotated with 
either Public(Evolving)/Internal. As it stands I cannot judge whether 
this is truly supposed to be an example / test utility or actually a 
user-facing class.
If this is to be used by connectors, which are included in the 
user-jar, then we're violating the principle above, in which case the 
class should be relocated/removed.


On 06.03.2019 15:10, Aljoscha Krettek wrote:

Hi,

I recently saw that we added a dependency on our shaded-guava to 
flink-core [1]. Just for the record, I don’t want do diminish the 
contributions of anyone involved in the PR in any way. It just made 
me realise that we have some implicit agreements or assumptions about 
adding certain things to core packages that we might never have 
really discussed. I think we should do that now.


Quite some time ago an effort was started to reduce our dependency on 
Guava [2] because of some problems with version stability and 
dependency conflicts. At some later point, we created shaded guava so 
that we could use it without clashes [3]. I believe we now have 
shaded Guava only in runtime modules where it wasn’t easy to remove 
and CEP.


With the creation of shaded guava, I think we are in a bit of a limbo 
situation where it is not exactly clear what our stance towards it 
is, because it is easy to add to modules, as evident by the 
aforementioned PR. I think we should discuss that situation and agree 
upon a common stance on the topic.


In general, I think the surface (which includes classes, interfaces, 
and dependencies, among other things) of core modules should be kept 
as lean as possible.  (all modules really)


What do you think?

Best,
Aljoscha

[1] https://github.com/apache/flink/pull/7679 

[2] https://issues.apache.org/jira/browse/FLINK-3700 


[3] https://issues.apache.org/jira/browse/FLINK-6982

Re: [DISCUSS] Using Guava in Flink "core" packages

[Chesnay Schepler]

I fully agree that we should write down this kind of conventions that 
committers have setup implicitly.


I agree that we should keep flink-core as lean as possible.

On guava usages in general my current stance is that we should only use 
guava if necessary. Spreading it (or other dependencies for that matter) 
to far just creates headaches when bumping the dependency.


One core principle that we *must* follow is that shaded dependencies are 
neither exposed to user-code nor contained in the user-jar. Otherwise we 
end up again in a situation where we cannot increment dependencies 
without breaking compatibility for users.


The PR at hand has a few issues in this regard. The guava limiter is 
only used in tests but is not a test class, yet isn't annotated with 
either Public(Evolving)/Internal. As it stands I cannot judge whether 
this is truly supposed to be an example / test utility or actually a 
user-facing class.
If this is to be used by connectors, which are included in the user-jar, 
then we're violating the principle above, in which case the class should 
be relocated/removed.


On 06.03.2019 15:10, Aljoscha Krettek wrote:

Hi,

I recently saw that we added a dependency on our shaded-guava to flink-core 
[1]. Just for the record, I don’t want do diminish the contributions of anyone 
involved in the PR in any way. It just made me realise that we have some 
implicit agreements or assumptions about adding certain things to core packages 
that we might never have really discussed. I think we should do that now.

Quite some time ago an effort was started to reduce our dependency on Guava [2] 
because of some problems with version stability and dependency conflicts. At 
some later point, we created shaded guava so that we could use it without 
clashes [3]. I believe we now have shaded Guava only in runtime modules where 
it wasn’t easy to remove and CEP.

With the creation of shaded guava, I think we are in a bit of a limbo situation 
where it is not exactly clear what our stance towards it is, because it is easy 
to add to modules, as evident by the aforementioned PR. I think we should 
discuss that situation and agree upon a common stance on the topic.

In general, I think the surface (which includes classes, interfaces, and 
dependencies, among other things) of core modules should be kept as lean as 
possible.  (all modules really)

What do you think?

Best,
Aljoscha

[1] https://github.com/apache/flink/pull/7679 

[2] https://issues.apache.org/jira/browse/FLINK-3700 

[3] https://issues.apache.org/jira/browse/FLINK-6982

[DISCUSS] Using Guava in Flink "core" packages

[Aljoscha Krettek]

Hi,

I recently saw that we added a dependency on our shaded-guava to flink-core 
[1]. Just for the record, I don’t want do diminish the contributions of anyone 
involved in the PR in any way. It just made me realise that we have some 
implicit agreements or assumptions about adding certain things to core packages 
that we might never have really discussed. I think we should do that now.

Quite some time ago an effort was started to reduce our dependency on Guava [2] 
because of some problems with version stability and dependency conflicts. At 
some later point, we created shaded guava so that we could use it without 
clashes [3]. I believe we now have shaded Guava only in runtime modules where 
it wasn’t easy to remove and CEP.

With the creation of shaded guava, I think we are in a bit of a limbo situation 
where it is not exactly clear what our stance towards it is, because it is easy 
to add to modules, as evident by the aforementioned PR. I think we should 
discuss that situation and agree upon a common stance on the topic.

In general, I think the surface (which includes classes, interfaces, and 
dependencies, among other things) of core modules should be kept as lean as 
possible.  (all modules really) 

What do you think?

Best,
Aljoscha

[1] https://github.com/apache/flink/pull/7679 

[2] https://issues.apache.org/jira/browse/FLINK-3700 

[3] https://issues.apache.org/jira/browse/FLINK-6982