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 > >>>>> > >>>>> > >>>>> > >>> > >>> > >
