Hi, Thanks for the follow-up.
@Aljoscha Krettek <aljos...@apache.org> 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 <se...@apache.org> 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 <aljos...@apache.org> > 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 <t...@apache.org> 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 <t...@apache.org> 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 <aljos...@apache.org> > > >> wrote: > > >> > > >>> I think the two links are identical. > > >>> > > >>>> On 6. Mar 2019, at 16:05, Thomas Weise <t...@apache.org> 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 <ches...@apache.org > > > > >>> 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 > > >>>>> > > >>>>> > > >>>>> > > >>> > > >>> > > > > >
