Follow up
On Mon, Aug 23, 2021 at 1:22 PM Andrey Gura <ag...@apache.org> wrote: > > 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
