The voting has been started here <https://lists.apache.org/thread.html/rfd7f449d5cfcffb728689ba4250a07a0df068d4ad1bdb677d855b8c6%40%3Cdev.ignite.apache.org%3E>
On Mon, Sep 6, 2021 at 8:11 PM Andrey Gura <ag...@apache.org> wrote: > Alexander, > > Feel free to start voting. Please, refer to this discussion in order > to avoid a new flaming thread in the voting topic. > > On Thu, Sep 2, 2021 at 6:04 PM Alexander Polovtcev > <alexpolovt...@gmail.com> wrote: > > > > Andrey, > > > > > It just doesn't work. If there is precedent of usage of Guava methods > > then somebody will use other methods. > > > > Maybe you are right. I would suggest a vote on whether we should allow > Guava methods in the codebase or not. Let's do that in a separate thread? > In the meantime we will prohibit using Guava until the voting is complete. > > > > On Thu, Sep 2, 2021 at 3:58 PM Andrey Gura <ag...@apache.org> wrote: > >> > >> > 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 > > > > > > > > -- > > With regards, > > Aleksandr Polovtcev > -- With regards, Aleksandr Polovtcev
