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
