I think since Calcite brings it in already then your arguments make sense. Would it be pinned to the same version as Calcite? Risk NoSuchMethodError at runtime if not. +1
On Mon, Aug 9, 2021 at 9:56 AM Alexander Polovtcev <alexpolovt...@gmail.com> wrote: > Zhenya, Courtney, Andrey, > > What do you think about my arguments, was I able to convince you? I would > like to reach some consensus here. At the moment, my original points still > stand, I'm also ok with shading Guava if needed, though I think it is not > necessary at this point. > > On Fri, Aug 6, 2021 at 12:45 PM Alexander Polovtcev < > alexpolovt...@gmail.com> > wrote: > > > Zhenya, > > > > > But there is no restrictions from running ignite server nodes from some > > other code with it`s own guava version seems we obtain fast path to jar > > hell here? > > > > I'm not sure if I fully understand your question, but it looks like we > are > > in this situation already, because we have some dependencies that use > > Guava. That's why I propose to add Guava explicitly to at least have a > > deterministic runtime configuration (see this link > > < > https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Management > > > > for an explanation). > > > > On Fri, Aug 6, 2021 at 12:25 PM Zhenya Stanilovsky > > <arzamas...@mail.ru.invalid> wrote: > > > >> > >> Alexander, first of all looks like Ivan Daschinsky approach about thin > >> client use only and shadow plugin are cover all Andrey Mashenkov listing > >> problems. > >> But there is no restrictions from running ignite server nodes from some > >> other code with it`s own guava version seems we obtain fast path to jar > >> hell here? > >> > >> > >> >Zhenya, > >> > > >> >My intentions are the following: > >> > > >> >1. Remove some copy-pasted code (like the "bytecode" module or some > >> utility > >> >methods). Please see my original message for the links to the code. > >> >2. Explicitly pin the Guava version to avoid conflicts in the runtime. > >> > > >> >About allowing to use Guava in the codebase, my thoughts are the > >> following: > >> > > >> >1. We *already* use some code from Guava either directly (like in the > >> >"calcite" module) or by copy-pasting it into a utility class. > >> >2. I understand that some Guava methods are obsolete as of Java 11, but > >> >some of them still don't have any standard library counterparts, in > which > >> >case I think using Guava is justified (which is supported by point 1). > >> > > >> >Can you please explain why you would disapprove of my proposal? > >> > > >> >On Thu, Aug 5, 2021 at 7:56 PM Zhenya Stanilovsky > >> >< arzamas...@mail.ru.invalid > wrote: > >> > > >> >> > >> >> alexpolovtcev please clarify what do you mean under : «possibility of > >> >> using Guava in Ignite 3», using how necessary dependency of calcite > or > >> >> using like «using in our code» ? If using in code, i -1 here. > >> >> thanks. > >> >> > >> >> > >> >> >Hello, dear Igniters! > >> >> > > >> >> >I would like to discuss the possibility of using Guava > >> >> >< https://github.com/google/guava > in Ignite 3. I know about the > >> >> restrictive > >> >> >policy of using it in Ignite 2, but I have the following reasons: > >> >> > > >> >> >1. We are de-facto using it already as an implicit dependency, since > >> the > >> >> >Calcite module depends on it, and Calcite is going to stay for a > >> while =) > >> >> >2. AFAIK, the "bytecode" module is copied into the codebase only to > >> strip > >> >> >Guava away from it. We can remove this module, which will improve > the > >> >> >maintainability of the project. > >> >> >3. We have some copy-paste of Guava code in the project. For > example, > >> see > >> >> >this > >> >> >< > >> >> > >> > https://github.com/apache/ignite-3/blob/main/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java#L136 > >> >> > > >> >> >and this > >> >> >< > >> >> > >> > https://github.com/apache/ignite-3/blob/main/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java#L428 > >> >> > > >> >> >. > >> >> >4. Regarding security concerns, this report > >> >> >< > >> >> > >> > https://www.cvedetails.com/product/52274/Google-Guava.html?vendor_id=1224 > >> >> > > >> >> >shows no major vulnerability issues for the last three years. > >> >> > > >> >> >Taking these points into account, I propose to allow using Guava > both > >> in > >> >> >production and test code and to add it as an explicit dependency. > >> >> > > >> >> >What do you think? > >> >> > > >> >> >-- > >> >> >With regards, > >> >> >Aleksandr Polovtcev > >> >> > >> >> > >> >> > >> >> > >> > > >> > > >> >-- > >> >With regards, > >> >Aleksandr Polovtcev > >> > >> > >> > >> > > > > > > > > -- > > With regards, > > Aleksandr Polovtcev > > > > > -- > With regards, > Aleksandr Polovtcev >
