Igniters, What I actually didn't understand from this thread: Is Guava allowed in production code of Ignite 3 modules (not dependencies like Calcite)?
While we agree with using shading I don't see any arguments about using Guava library in our code base except for the fact that we have some copy-paste of Guava code in the project. Guava is rich enough library and it has both advantages and disadvantages. Ignite code base always strived to be not overloaded and minimalistic. For example we usually use immutability very rare, we try to avoid complex and overloaded APIs on hot path, we try reduce GC pressure, etc. In turn, Guava is some sense forces using of immutable collections, has a lot of utility methods which produces temp object (yes, I don't trust to escap analysys), etc. Rich set of collections and utility functions will also lead to different programming styles during Ignite development. I can't predict it, but it is usual enough when some developers force immutablity while others remove static type declaration and replace it by var :) This is a common pratice to reduce language possibilies subset and limiting using of some libraries in order to provide more or less unified approach to coding. So I propose to disallow using Guava in our code base. WDYT? On Fri, Aug 20, 2021 at 7:37 PM Alexander Polovtcev <alexpolovt...@gmail.com> wrote: > > Guys, > > Thanks again for your responses. I've created a ticket about using the > Shade plugin in order to understand if it is possible to shade and minimize > Guava, I will start working on it shortly: > https://issues.apache.org/jira/browse/IGNITE-15354 > > On Tue, Aug 10, 2021 at 1:51 PM Courtney Robinson <courtney.robin...@hypi.io> > wrote: > > > 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 > > > > > > > > -- > With regards, > Aleksandr Polovtcev
