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

Reply via email to