Hi Uday, We should keep classes name as is within all minor releases (2.x). We can schedule rename only to 3.0. We need to keep both classes and methods naming as is to provide compilation guarantee. If someone used existing TcpDiscoveryElbIpFinder from 2.6 release than his or her code should compile and run well.
I've updated checklist, thank you for pointing to this. So I suggest keeping existing naming of TcpDiscoveryElbIpFinder as it is part of API. Sincerely, Dmitriy Pavlov вт, 2 окт. 2018 г. в 21:04, uday kale <udaygk...@gmail.com>: > Thanks for the review Stan. > I renamed the existing class TcpDiscoveryElbIpFinder since the name is not > clear regarding the actual load balancer being used. The names > TcpDiscoveryClassicElbIpFinder > and TcpDiscoveryApplicationElbIpFinder provide a clear distinction. > I deprecated the existing class because of review checklist [1] point 1.a > under checklist. It requires methods to be deprecated instead of being > renamed. I assumed this will extend to classes too. > Regarding the your suggestion for having the tests I agree. It will be > helpful to know whether TC can accept properties while initiating a run. > > [1] https://cwiki.apache.org/confluence/display/IGNITE/Review+Checklist > > Uday > > On Tue, Oct 2, 2018 at 9:58 PM Stanislav Lukyanov <stanlukya...@gmail.com> > wrote: > > > Also, it makes me nervous that we’re lacking any sort of functional tests > > for AWS-based IP finders (same for GCE, actually). > > I understand that it’s hard to include tests like that in regular TC runs > > because we’d have to include some sort of credentials. > > But let’s think of some sort of a middle ground. > > > > I’m thinking about a functional test suite in the ignite-aws module that > > would pick up a properties file with AWS credentials. > > It wouldn’t be included in any of the TC runs, but someone making changes > > to ignite-aws would be able to run it locally providing their own > > credentials. > > They’d then share the results of their testing together with the usual TC > > link. > > > > Stan > > > > From: Stanislav Lukyanov > > Sent: 2 октября 2018 г. 19:08 > > To: dev@ignite.apache.org > > Subject: RE: Volunteer needed: AWS Elastic Load Balancer IP > > Findersimplemented > > > > Hi, > > > > I took a look at the code, and I believe the patch needs to be enhanced. > > > > The patch is about adding TcpDiscoveryApplicationElbIpFinder, but it also > > deprecates the existing TcpDiscoveryElbIpFinder > > and replaces it with its copy named TcpDiscoveryClassicElbIpFinder. > > > > I’d really prefer if the existing class were enhanced to support both > > classic ELB and Application ELB. > > If it can’t be done, let’s use inheritance to reduce the copypaste. > > In any case, let’s avoid deprecating the existing class – I don’t think > > that changing the name is that important in this case. > > > > Thanks, > > Stan > > > > From: Denis Magda > > Sent: 26 сентября 2018 г. 18:52 > > To: dev > > Subject: Re: Volunteer needed: AWS Elastic Load Balancer IP > > Findersimplemented > > > > Igniters, > > > > Don't we have experts in our networking component? I do believe that's a > > small improvement that can be verified and merged quickly. > > > > -- > > Denis > > > > On Wed, Sep 26, 2018 at 8:50 AM Dmitriy Pavlov <dpavlov....@gmail.com> > > wrote: > > > > > Igniters, Friendly reminder. > > > > > > Denis, could you advice how to proceed in this case? It seems feature > can > > > be useful, but committers are not involved in a review. > > > > > > Should we send a proposal with lazy consensus and automatically merge? > > Any > > > alternative ideas on how to proceed with issues stuck in the review > > process > > > are strongly appreciated. > > > > > > Sincerely, > > > Dmitriy Pavlov > > > > > > ср, 15 авг. 2018 г. в 19:38, Dmitriy Pavlov <dpavlov....@gmail.com>: > > > > > > > Hi Igniters, > > > > > > > > I took a look at the proposed contribution > > > > https://issues.apache.org/jira/browse/IGNITE-8617 and PR > > > > https://github.com/apache/ignite/pull/4076. > > > > > > > > I found this change reasonable and I can update code according to > code > > > > style. > > > > > > > > But I would ask someone from the community with experience with AWS > ELB > > > to > > > > join the review. It would also benefit us if someone could try to > run a > > > > cluster with AWS ELB IP Finder. > > > > > > > > Sincerely, > > > > Dmitriy Pavlov > > > > > > > > > > > > > >
