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