> So I would propose to allow at least *some* methods that we consider useful, > while disallowing everything else.
It just doesn't work. If there is precedent of usage of Guava methods then somebody will use other methods. > I think that the benefits of using Guava methods instead of copy-pasting them > are quite obvious: you don't have to copy-paste code and support it I don't understand why somebody did such a copy-paste, but I believe that nobody supports this code. So, from my point of view, it isn't a problem. On Thu, Sep 2, 2021 at 2:52 PM Alexander Polovtcev <alexpolovt...@gmail.com> wrote: > > Andrey, > I think that the benefits of using Guava methods instead of copy-pasting them > are quite obvious: you don't have to copy-paste code and support it. I also > find this situation quite strange: we have a dependency and copy-paste code > from it instead of using it directly. > > > On Thu, Sep 2, 2021 at 1:58 PM Andrey Mashenkov <andrey.mashen...@gmail.com> > wrote: >> >> 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 > > > > -- > With regards, > Aleksandr Polovtcev
