Alex, As I understand we agree to shade Guava transitive dependency and you said a 'maven-shade-plugin' can drop unused Guava methods to reduce the footprint of Ignite jar.
At this point, there is no difference between copy-pasting Guava method to Ignite and use Guava one. The resulted jar will have one copy of such method, but in the second case, we have to care about compatibility when the transitive Guava dependency will be updated to a new version. So, I see no benefits from using Guava directly. On Thu, Sep 2, 2021 at 11:56 AM Alexander Polovtcev <alexpolovt...@gmail.com> wrote: > Hi, Andrey! > I mostly agree with your proposal, but, since we already have some > copy-paste in our code, can we at least use Guava to remove it? So I would > propose to allow at least *some* methods that we consider useful, while > disallowing everything else. I understand that it may be difficult to > formalize, so maybe we can create some kind of a whitelist of Guava > methods? What do you think? > > On Mon, Aug 30, 2021 at 2:54 PM Andrey Gura <ag...@apache.org> wrote: > > > 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 > > > > > -- > With regards, > Aleksandr Polovtcev > -- Best regards, Andrey V. Mashenkov
