Yes, currently handler handles exception in the same way as 'GridAbstractTest', so, it will work from any thread.
пт, 11 янв. 2019 г., 15:18 Ilya Lantukh [email protected]: > Dmitry, > > It doesn't make sense to run that test now because the root cause of it's > failure had been fixed. > > You should verify that getting an unhandled exception in any system thread > leads to the failure of currently running test. > > On Fri, Jan 11, 2019 at 12:16 PM Dmitrii Ryabov <[email protected]> > wrote: > >> Ilya, can you check your test on current implementation [1]? >> >> [1] https://github.com/apache/ignite/pull/5662 >> >> 10 дек. 2018 г. 17:10 пользователь "Dmitriy Pavlov" <[email protected]> >> написал: >> >> Reverted. >> >> https://issues.apache.org/jira/browse/IGNITE-8227 reopened >> >> пн, 10 дек. 2018 г. в 16:23, Dmitriy Pavlov <[email protected]>: >> >> >> > Anton, I was expecting that you revert, because you wanted to do it. >> > >> > Provided that I agree that fix could be reverted because of both >> > functional and style possible improvements, does not mean I believe it >> is >> > the only option and it should be reverted. >> > >> > Even if I agree to revert doesn't mean all community agrees, so >> reverting >> > just 1 minute after writing to dev list would be strange. I believe we >> > should be courteous enough to give a couple of days for people to come >> and >> > give feedback. >> > >> > So if you have a spare minute, please go ahead. If not, I can do it >> later. >> > >> > пн, 10 дек. 2018 г. в 14:23, Anton Vinogradov <[email protected]>: >> > >> >> Dmitriy, >> >> >> >> You confirmed that fix should be reverted and reworked last Friday. >> >> Why it still not reverted? >> >> >> >> On Mon, Dec 10, 2018 at 12:46 AM Dmitrii Ryabov <[email protected] >> > >> >> wrote: >> >> >> >> > Agree, it is reasonable to revert. >> >> > пт, 7 дек. 2018 г. в 18:44, Dmitriy Pavlov <[email protected]>: >> >> > > >> >> > > Hi Ilya, >> >> > > >> >> > > thank you for noticing. >> >> > > >> >> > > Calling to fail is equal to re-throw, >> >> > > >> >> > > throw new AssertionFailedError(message); >> >> > > >> >> > > So, yes, for now it is absolutely valid reason to revert and rework >> >> fix >> >> > > >> >> > > - as Nikolay suggested to reduce method override ocurrences. >> >> > > - and with transferring this exception into GridAbstractTest and >> >> > > correctly failing test. >> >> > > >> >> > > Sincerely, >> >> > > Dmitriy Pavlov >> >> > > >> >> > > >> >> > > пт, 7 дек. 2018 г. в 18:38, Ilya Lantukh <[email protected]>: >> >> > > >> >> > > > Unfortunately, this FailureHandler doesn't seem to work. I wrote >> a >> >> test >> >> > > > that reproduces a bug and should fail. It prints the following >> text >> >> > into >> >> > > > log, but the test still passes "successfully": >> >> > > > >> >> > > > [2018-12-07 >> >> > > > >> >> > > > >> >> > >> >> >> 18:28:23,800][ERROR][sys-stripe-1-#345%recovery.GridPointInTimeRecoveryCacheNoAffinityExchangeTest1%][IgniteTestResources] >> >> > > > Critical system error detected. Will be handled accordingly to >> >> > configured >> >> > > > handler [hnd=TestFailingFailureHandler [], >> failureCtx=FailureContext >> >> > > > [type=CRITICAL_ERROR, err=java.lang.IllegalStateException: >> Unable to >> >> > find >> >> > > > consistentId by UUID >> [nodeId=80dd2ec6-1913-4a5c-a839-630315c00003, >> >> > > > topVer=AffinityTopologyVersion [topVer=12, minorTopVer=0]]]] >> >> > > > java.lang.IllegalStateException: Unable to find consistentId by >> UUID >> >> > > > [nodeId=80dd2ec6-1913-4a5c-a839-630315c00003, >> >> > > > topVer=AffinityTopologyVersion [topVer=12, minorTopVer=0]] >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.managers.discovery.ConsistentIdMapper.mapToCompactId(ConsistentIdMapper.java:62) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.managers.discovery.ConsistentIdMapper.mapToCompactIds(ConsistentIdMapper.java:123) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.processors.cache.transactions.IgniteTxManager.newTxRecord(IgniteTxManager.java:2507) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.processors.cache.transactions.IgniteTxManager.logTxRecord(IgniteTxManager.java:2483) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.processors.cache.transactions.IgniteTxAdapter.state(IgniteTxAdapter.java:1226) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.processors.cache.transactions.IgniteTxAdapter.state(IgniteTxAdapter.java:1054) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.processors.cache.transactions.IgniteTxHandler.startRemoteTx(IgniteTxHandler.java:1836) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.processors.cache.transactions.IgniteTxHandler.processDhtTxPrepareRequest(IgniteTxHandler.java:1180) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.processors.cache.transactions.IgniteTxHandler.access$400(IgniteTxHandler.java:118) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.processors.cache.transactions.IgniteTxHandler$5.apply(IgniteTxHandler.java:222) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.processors.cache.transactions.IgniteTxHandler$5.apply(IgniteTxHandler.java:220) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.processors.cache.GridCacheIoManager.processMessage(GridCacheIoManager.java:1059) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.processors.cache.GridCacheIoManager.onMessage0(GridCacheIoManager.java:584) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.processors.cache.GridCacheIoManager.handleMessage(GridCacheIoManager.java:383) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.processors.cache.GridCacheIoManager.handleMessage(GridCacheIoManager.java:309) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.processors.cache.GridCacheIoManager.access$100(GridCacheIoManager.java:100) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.processors.cache.GridCacheIoManager$1.onMessage(GridCacheIoManager.java:299) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.managers.communication.GridIoManager.invokeListener(GridIoManager.java:1568) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.managers.communication.GridIoManager.processRegularMessage0(GridIoManager.java:1196) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.managers.communication.GridIoManager.access$4200(GridIoManager.java:127) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.managers.communication.GridIoManager$9.run(GridIoManager.java:1092) >> >> > > > at >> >> > > > >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.util.StripedExecutor$Stripe.body(StripedExecutor.java:505) >> >> > > > at >> >> > > > >> >> > >> >> >> org.apache.ignite.internal.util.worker.GridWorker.run(GridWorker.java:120) >> >> > > > at java.lang.Thread.run(Thread.java:748) >> >> > > > >> >> > > > >> >> > > > On Thu, Dec 6, 2018 at 4:01 PM Anton Vinogradov <[email protected]> >> >> wrote: >> >> > > > >> >> > > > > >> We stop, for now, then you will chill a >> >> > > > > >> little bit, then you will have an absolutely fantastic >> weekend, >> >> > and >> >> > > > then >> >> > > > > on >> >> > > > > >> Monday, Dec 10 we will continue this discussion in a >> positive >> >> and >> >> > > > > >> constructive manner. >> >> > > > > Agree >> >> > > > > >> >> > > > > On Thu, Dec 6, 2018 at 3:55 PM Nikolay Izhikov < >> >> [email protected]> >> >> > > > > wrote: >> >> > > > > >> >> > > > > > Anton. >> >> > > > > > >> >> > > > > > I discussed this fix privately with Dmitriy Pavlov. >> >> > > > > > >> >> > > > > > 1. We had NoOpHandler for ALL tests before this merge. >> >> > > > > > 2. Dmitry Ryabov will remove all copypasted code soon. >> >> > > > > > >> >> > > > > > So, this fix make things better. >> >> > > > > > >> >> > > > > > I think we shouldn't revert it. >> >> > > > > > >> >> > > > > > I think we should continue work to turn off NoOpHandler in >> all >> >> > tests. >> >> > > > > > >> >> > > > > > Dmitriy Pavlov, can you do it, as a committer of this patch? >> >> > > > > > >> >> > > > > > On 12/6/18 3:02 PM, Anton Vinogradov wrote: >> >> > > > > > >>> I still hope Anton will do the first bunch of tests >> >> research to >> >> > > > > > > demonstrate >> >> > > > > > >>> the idea. >> >> > > > > > > >> >> > > > > > > Dmitriy, >> >> > > > > > > Just want to remind you that we already spend time here >> >> because >> >> > of >> >> > > > > > > unacceptable code merge situation. >> >> > > > > > > Such merges should NEVER happen again. >> >> > > > > > > Please, next time make sure that code you merge has no >> massive >> >> > > > > > duplication >> >> > > > > > > and fixes without proper reason investigation. >> >> > > > > > > Committer always MUST be ready to explain each symbol >> inside >> >> > code he >> >> > > > > > merged. >> >> > > > > > > The situation when you have no clue why it written this way >> >> > > > > unacceptable. >> >> > > > > > > >> >> > > > > > > Feel free to start a discussion at private in case you have >> >> some >> >> > > > > > objections. >> >> > > > > > > But, hope you agree and will help us to solve the issue >> >> instead. >> >> > > > > > > >> >> > > > > > > Dmitrii, >> >> > > > > > >>> Anton, I mean `copy-paste reduce` ticket. I'll try to >> >> describe >> >> > the >> >> > > > > > > reasons for >> >> > > > > > >>> no-op in tests. Then, we can create tickets to fix this >> >> cases >> >> > if >> >> > > > > > needed. >> >> > > > > > > >> >> > > > > > > In case no-one will be ready to start a proper fix >> >> (investigate >> >> > why >> >> > > > > every >> >> > > > > > > no-op required and create tickets for each problem) before >> >> Friday >> >> > > > > > evening, >> >> > > > > > > the code will be rolled back. >> >> > > > > > > Simple no-op is better that same but overcomplicated. >> >> > > > > > > >> >> > > > > > > On Thu, Dec 6, 2018 at 2:14 PM Dmitrii Ryabov < >> >> > [email protected] >> >> > > > > >> >> > > > > > wrote: >> >> > > > > > > >> >> > > > > > >> Anton, I mean `copy-paste reduce` ticket. I'll try to >> >> describe >> >> > > > reasons >> >> > > > > > for >> >> > > > > > >> no-op in tests. Then, we can create tickets to fix this >> >> cases if >> >> > > > > needed. >> >> > > > > > >> >> >> > > > > > >> чт, 6 дек. 2018 г., 13:53 Dmitriy Pavlov >> [email protected]: >> >> > > > > > >> >> >> > > > > > >>> BTW, No-Op or StopNode-FailTest in case of a deep >> >> investigation >> >> > > > will >> >> > > > > > >> always >> >> > > > > > >>> require to understand what test does and what it tests. >> >> > > > > > >>> >> >> > > > > > >>> So we can get a positive outcome from this research if we >> >> > agree to >> >> > > > > add >> >> > > > > > >>> - a small description to each test about the reason for >> >> > existing of >> >> > > > > > this >> >> > > > > > >>> test, >> >> > > > > > >>> - what is the expected behavior of the product in the >> test, >> >> > and how >> >> > > > > it >> >> > > > > > is >> >> > > > > > >>> checked? >> >> > > > > > >>> - failure handler influence, etc. >> >> > > > > > >>> >> >> > > > > > >>> I still hope Anton will do the first bunch of tests >> >> research to >> >> > > > > > >> demonstrate >> >> > > > > > >>> the idea. >> >> > > > > > >>> >> >> > > > > > >>> чт, 6 дек. 2018 г. в 13:39, Anton Vinogradov < >> [email protected] >> >> >: >> >> > > > > > >>> >> >> > > > > > >>>> Dmitrii, >> >> > > > > > >>>> >> >> > > > > > >>>>>> I agree with Nikolay's solution. If no one minds, I'll >> >> > create >> >> > > > > ticket >> >> > > > > > >>> for >> >> > > > > > >>>>>> appropriate changes and recheck issues. >> >> > > > > > >>>> Do you mean 'copy-paste reduce' ticket or check/fix of >> all >> >> > tests >> >> > > > > with >> >> > > > > > >>> no-op >> >> > > > > > >>>> to have a proper handler? >> >> > > > > > >>>> >> >> > > > > > >>>> Just want to make sure that copy-paste minimization is >> not >> >> the >> >> > > > final >> >> > > > > > >>> step. >> >> > > > > > >>>> >> >> > > > > > >>>> On Thu, Dec 6, 2018 at 1:24 PM Павлухин Иван < >> >> > [email protected] >> >> > > > > >> >> > > > > > >>> wrote: >> >> > > > > > >>>> >> >> > > > > > >>>>> Dmitrii Ryabov, >> >> > > > > > >>>>> >> >> > > > > > >>>>> Your comments sounds reasonable to me. Marker base >> class >> >> > approach >> >> > > > > > >>>>> looks good to me so far. >> >> > > > > > >>>>> >> >> > > > > > >>>>> P.S. I had even worse name in mind 'StopGaps' =) >> >> > > > > > >>>>> чт, 6 дек. 2018 г. в 13:08, Dmitrii Ryabov < >> >> > > > [email protected] >> >> > > > > >: >> >> > > > > > >>>>>> >> >> > > > > > >>>>>> Ivan, I think `Workarounds` class isn't good idea, >> >> because >> >> > it >> >> > > > > looks >> >> > > > > > >>>> like >> >> > > > > > >>>>> we >> >> > > > > > >>>>>> create stable workarounds, which will never be fixed. >> >> > > > > > >>>>>> >> >> > > > > > >>>>>> I agree with Nikolay's solution. If no one minds, I'll >> >> > create >> >> > > > > > >> ticket >> >> > > > > > >>>> for >> >> > > > > > >>>>>> appropriate changes and recheck issues. >> >> > > > > > >>>>>> >> >> > > > > > >>>>>> чт, 6 дек. 2018 г., 12:17 Anton Vinogradov >> [email protected] >> >> : >> >> > > > > > >>>>>> >> >> > > > > > >>>>>>> Folks, thank's everyone for solution research. >> >> > > > > > >>>>>>> I'm ok with Nikolay approach in case that's not a >> final >> >> > step. >> >> > > > > > >>>>>>> >> >> > > > > > >>>>>>> On Thu, Dec 6, 2018 at 12:11 PM Павлухин Иван < >> >> > > > > > >> [email protected] >> >> > > > > > >>>> >> >> > > > > > >>>>> wrote: >> >> > > > > > >>>>>>> >> >> > > > > > >>>>>>>> Nikolay, >> >> > > > > > >>>>>>>> >> >> > > > > > >>>>>>>> I meant "not expensive" by "cheap". And I meant that >> >> it is >> >> > > > good >> >> > > > > > >>>> that >> >> > > > > > >>>>>>>> it cheap =). And I said it to contrast with >> "expensive" >> >> > ~100 >> >> > > > > > >>> tests >> >> > > > > > >>>>>>>> investigation. And if we agree (mostly I would like >> an >> >> > opinion >> >> > > > > > >>> from >> >> > > > > > >>>>>>>> Dmitriy Ryabov as an original author) on a way how >> to >> >> > improve >> >> > > > > > >> the >> >> > > > > > >>>>>>>> patch then let's do it. >> >> > > > > > >>>>>>>> чт, 6 дек. 2018 г. в 10:41, Nikolay Izhikov < >> >> > > > > > >> [email protected] >> >> > > > > > >>>> : >> >> > > > > > >>>>>>>>> >> >> > > > > > >>>>>>>>> Dmitriy Ryabov, Dmitriy Pavlov, sorry. >> >> > > > > > >>>>>>>>> >> >> > > > > > >>>>>>>>> Of course it should be "NOT to blame author". >> >> > > > > > >>>>>>>>> >> >> > > > > > >>>>>>>>> Sorry, one more time. >> >> > > > > > >>>>>>>>> >> >> > > > > > >>>>>>>>> чт, 6 дек. 2018 г., 10:40 Dmitriy Pavlov >> >> > [email protected]: >> >> > > > > > >>>>>>>>> >> >> > > > > > >>>>>>>>>> I hope you've misprinted here >> >> > > > > > >>>>>>>>>>> I'm here to blame the author. >> >> > > > > > >>>>>>>>>> >> >> > > > > > >>>>>>>>>> We can blame code but never coders. >> >> > > > > > >>>>>>>>>> >> >> > > > > > >>>>>>>>>> Please see https://discourse.pi-hole.net/faq - >> has >> >> > > > > > >>> absolutely >> >> > > > > > >>>>>>> nothing >> >> > > > > > >>>>>>>> in >> >> > > > > > >>>>>>>>>> common with Apache Guides, but says the same >> things. >> >> It >> >> > is >> >> > > > > > >> a >> >> > > > > > >>>>>>> practical >> >> > > > > > >>>>>>>>>> necessity to maintain a friendly atmosphere. >> >> > > > > > >>>>>>>>>> >> >> > > > > > >>>>>>>>>> чт, 6 дек. 2018 г. в 10:31, Nikolay Izhikov < >> >> > > > > > >>>> [email protected] >> >> > > > > > >>>>>> : >> >> > > > > > >>>>>>>>>> >> >> > > > > > >>>>>>>>>>> Ivan. >> >> > > > > > >>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>> 1. Accept the patch and bring an improvement to >> >> Ignite >> >> > > > > > >>> (and >> >> > > > > > >>>>>>> create >> >> > > > > > >>>>>>>> a> >> >> > > > > > >>>>>>>>>>> ticket for further investigation). >> >> > > > > > >>>>>>>>>>> >> >> > > > > > >>>>>>>>>>> I support this idea. >> >> > > > > > >>>>>>>>>>> Do we create the tickets already? >> >> > > > > > >>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>> Nikolay's patch [1] suggests a slightly >> different >> >> > > > > > >>> approach >> >> > > > > > >>>>> how to >> >> > > > > > >>>>>>>> the >> >> > > > > > >>>>>>>>>>>> same thing. And implementing that idea looks >> like a >> >> > > > > > >> cheap >> >> > > > > > >>>>>>>> refactoring. >> >> > > > > > >>>>>>>>>>> >> >> > > > > > >>>>>>>>>>> I don't agree with your term "cheap". >> >> > > > > > >>>>>>>>>>> Do you think reducing copy paste code not worth >> it? >> >> > > > > > >>>>>>>>>>> >> >> > > > > > >>>>>>>>>>> I see a hundreds issues that bring copypasted >> code >> >> in >> >> > the >> >> > > > > > >>>>>>>> product(Ignite >> >> > > > > > >>>>>>>>>>> and others). >> >> > > > > > >>>>>>>>>>> I insist, that we shouldn't accept patches with >> it. >> >> > > > > > >>>>>>>>>>> >> >> > > > > > >>>>>>>>>>> I'm here to blame the author. >> >> > > > > > >>>>>>>>>>> I want to improve this patch and make it easier >> to >> >> find >> >> > > > > > >> all >> >> > > > > > >>>>> places >> >> > > > > > >>>>>>>> with >> >> > > > > > >>>>>>>>>>> NoOp handler to do the further investigation. >> >> > > > > > >>>>>>>>>>> >> >> > > > > > >>>>>>>>>>> В Чт, 06/12/2018 в 10:19 +0300, Павлухин Иван >> пишет: >> >> > > > > > >>>>>>>>>>>> Guys, >> >> > > > > > >>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>> I asked what harm will applying the patch bring >> I >> >> have >> >> > > > > > >>> not >> >> > > > > > >>>>> got a >> >> > > > > > >>>>>>>>>>>> direct answer. But I think I got some pain >> points: >> >> > > > > > >>>>>>>>>>>> 1. Anton does not like that reasons why ~100 >> tests >> >> > > > > > >>> require >> >> > > > > > >>>>> noop >> >> > > > > > >>>>>>>>>>>> handler are not clear. And might be several >> >> problems >> >> > > > > > >> are >> >> > > > > > >>>>> covered >> >> > > > > > >>>>>>>>>>>> there. >> >> > > > > > >>>>>>>>>>>> 2. Nikolay suggests some code improvements. >> >> > > > > > >>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>> Nikolay's patch [1] suggests a slightly >> different >> >> > > > > > >>> approach >> >> > > > > > >>>>> how to >> >> > > > > > >>>>>>>> the >> >> > > > > > >>>>>>>>>>>> same thing. And implementing that idea looks >> like a >> >> > > > > > >> cheap >> >> > > > > > >>>>>>>> refactoring. >> >> > > > > > >>>>>>>>>>>> But the idea of course could be discussed. >> Straight >> >> > > > > > >> away >> >> > > > > > >>> I >> >> > > > > > >>>>> can >> >> > > > > > >>>>>>>> suggest >> >> > > > > > >>>>>>>>>>>> another slightly different trick [2]. >> >> > > > > > >>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>> Investigating why ~100 tests require noop >> handler >> >> > could >> >> > > > > > >>> be >> >> > > > > > >>>>>>> costly. >> >> > > > > > >>>>>>>> So, >> >> > > > > > >>>>>>>>>>>> in that direction I see following options which >> can >> >> > > > > > >>> happen >> >> > > > > > >>>>> for >> >> > > > > > >>>>>>>> sure: >> >> > > > > > >>>>>>>>>>>> 1. Accept the patch and bring an improvement to >> >> Ignite >> >> > > > > > >>> (and >> >> > > > > > >>>>>>> create >> >> > > > > > >>>>>>>> a >> >> > > > > > >>>>>>>>>>>> ticket for further investigation). >> >> > > > > > >>>>>>>>>>>> 2. Revert the patch and loose an improvement. >> >> > > > > > >>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>> One might say that there is an option "Revert >> the >> >> > patch >> >> > > > > > >>> and >> >> > > > > > >>>>> then >> >> > > > > > >>>>>>>> do it >> >> > > > > > >>>>>>>>>>>> better" but I does not see anything (anyone) >> what >> >> can >> >> > > > > > >>>>> guarantee >> >> > > > > > >>>>>>> it. >> >> > > > > > >>>>>>>>>>>> So, I personally prefer an option 1 against 2 >> >> because >> >> > I >> >> > > > > > >>>>> believe >> >> > > > > > >>>>>>>> that >> >> > > > > > >>>>>>>>>>>> it is good if the system "can make a progress". >> >> > > > > > >>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>> [1] >> >> https://github.com/apache/ignite/pull/5584/files >> >> > > > > > >>>>>>>>>>>> [2] >> >> https://github.com/apache/ignite/pull/5586/files >> >> > > > > > >>>>>>>>>>>> ср, 5 дек. 2018 г. в 21:22, Nikolay Izhikov < >> >> > > > > > >>>>> [email protected] >> >> > > > > > >>>>>>>> : >> >> > > > > > >>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>> Dmitriy. >> >> > > > > > >>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>> The closest analog to Noop handler is mute of >> >> test >> >> > > > > > >>>>> failure. >> >> > > > > > >>>>>>>>>>>>>> By this commit, we had unmuted (possible) >> >> failures >> >> > > > > > >> in >> >> > > > > > >>>>>>>>>>> ~50000-~100=~49900 >> >> > > > > > >>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>> tests, and we’re still concerned about style or >> >> minor >> >> > > > > > >>>>> details >> >> > > > > > >>>>>>> if >> >> > > > > > >>>>>>>>>> no-op >> >> > > > > > >>>>>>>>>>> was >> >> > > > > > >>>>>>>>>>>>> copy-pasted, aren’t we? >> >> > > > > > >>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>> Can you explain this idea a bit more? >> >> > > > > > >>>>>>>>>>>>> I don't understand what is unmuted by discussed >> >> > > > > > >> commit. >> >> > > > > > >>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>> ср, 5 дек. 2018 г. в 20:40, Nikolay Izhikov < >> >> > > > > > >>>>>>> [email protected] >> >> > > > > > >>>>>>>>> : >> >> > > > > > >>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> Thanks, as an improvement to the code, this >> may >> >> > > > > > >> be >> >> > > > > > >>>>> better. >> >> > > > > > >>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>> I can prepare a full patch for NoOp handler. >> >> > > > > > >>>>>>>>>>>>>> What do you think? >> >> > > > > > >>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>> Anton Vinogradov, do you agree with this >> >> approach? >> >> > > > > > >>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>> ср, 5 дек. 2018 г. в 20:33, Dmitriy Pavlov < >> >> > > > > > >>>>>>> [email protected] >> >> > > > > > >>>>>>>>> : >> >> > > > > > >>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> Thanks, as an improvement to the code, this >> may >> >> > > > > > >> be >> >> > > > > > >>>>> better. >> >> > > > > > >>>>>>>> But >> >> > > > > > >>>>>>>>>>> still, it >> >> > > > > > >>>>>>>>>>>>>>> is >> >> > > > > > >>>>>>>>>>>>>>> not a reason to revert. And Anton mentioned >> >> > > > > > >>> something >> >> > > > > > >>>>> with >> >> > > > > > >>>>>>>> better >> >> > > > > > >>>>>>>>>>>>>>> exception >> >> > > > > > >>>>>>>>>>>>>>> handling/logging. Probably we will see an >> >> > > > > > >>>>> implementation as >> >> > > > > > >>>>>>>> well. >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> This case here is a big thing related to The >> >> > > > > > >> Apache >> >> > > > > > >>>>> Way, - >> >> > > > > > >>>>>>>> and >> >> > > > > > >>>>>>>>>> I'll >> >> > > > > > >>>>>>>>>>>>>>> explain >> >> > > > > > >>>>>>>>>>>>>>> why it makes me switched into fight-mode - >> until >> >> > > > > > >> we >> >> > > > > > >>>>> stop >> >> > > > > > >>>>>>> this >> >> > > > > > >>>>>>>>>>> nonsense. If >> >> > > > > > >>>>>>>>>>>>>>> PMCs (at least) are aware of patterns and >> >> > > > > > >>>>> anti-patterns in >> >> > > > > > >>>>>>>> the >> >> > > > > > >>>>>>>>>>> community, >> >> > > > > > >>>>>>>>>>>>>>> we will succeed as a project much more as >> with >> >> > > > > > >>> (only) >> >> > > > > > >>>>>>> perfect >> >> > > > > > >>>>>>>>>> code. >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> The closest analog to Noop handler is mute of >> >> > > > > > >> test >> >> > > > > > >>>>> failure. >> >> > > > > > >>>>>>>> By >> >> > > > > > >>>>>>>>>> this >> >> > > > > > >>>>>>>>>>>>>>> commit, >> >> > > > > > >>>>>>>>>>>>>>> we had unmuted (possible) failures in >> >> > > > > > >>>>> ~50000-~100=~49900 >> >> > > > > > >>>>>>>> tests, >> >> > > > > > >>>>>>>>>>> and we’re >> >> > > > > > >>>>>>>>>>>>>>> still concerned about style or minor details >> if >> >> > > > > > >>> no-op >> >> > > > > > >>>>> was >> >> > > > > > >>>>>>>>>>> copy-pasted, >> >> > > > > > >>>>>>>>>>>>>>> aren’t we? >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> To everyone arguing about the number of >> tests we >> >> > > > > > >>> are >> >> > > > > > >>>>>>> allowed >> >> > > > > > >>>>>>>> to >> >> > > > > > >>>>>>>>>>> have with >> >> > > > > > >>>>>>>>>>>>>>> no-op: please visit this page >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>> >> >> > > > > > >>>>>>>>>> >> >> > > > > > >>>>>>>> >> >> > > > > > >>>>>>> >> >> > > > > > >>>>> >> >> > > > > > >>>> >> >> > > > > > >>> >> >> > > > > > >> >> >> > > > > > >> >> > > > > > >> >> > > > > >> >> > > > >> >> > >> >> >> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8&tab=mutedProblems&branch_IgniteTests24Java8=__all_branches__ >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> It says: Muted tests: 3154. Are there any >> >> > > > > > >>>> disagreements >> >> > > > > > >>>>>>>> here? Why >> >> > > > > > >>>>>>>>>>> there >> >> > > > > > >>>>>>>>>>>>>>> are >> >> > > > > > >>>>>>>>>>>>>>> no insistent disagreement/not happy PMCs with >> >> > > > > > >>>>> absolutely >> >> > > > > > >>>>>>>>>>> unconditionally >> >> > > > > > >>>>>>>>>>>>>>> muted failures? >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> Any reason now to continue the discussion >> about >> >> > > > > > >>>>> reverting >> >> > > > > > >>>>>>>>>>> absolutely >> >> > > > > > >>>>>>>>>>>>>>> positive contribution into product stability >> >> from >> >> > > > > > >>>>> Dmitrii >> >> > > > > > >>>>>>> R.? >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> Moreover, Dmitrii Ryabov is trying to solve >> odd >> >> > > > > > >>> mutes >> >> > > > > > >>>>>>>> problem, as >> >> > > > > > >>>>>>>>>>> well, to >> >> > > > > > >>>>>>>>>>>>>>> locate mutes with links resolved issues in >> the >> >> TC >> >> > > > > > >>>> Bot. >> >> > > > > > >>>>> Is >> >> > > > > > >>>>>>> he >> >> > > > > > >>>>>>>>>>> deserved to >> >> > > > > > >>>>>>>>>>>>>>> read denouncing comments about the >> contribution? >> >> > > > > > >> I >> >> > > > > > >>>>> guess, >> >> > > > > > >>>>>>> no, >> >> > > > > > >>>>>>>>>>> especially >> >> > > > > > >>>>>>>>>>>>>>> if >> >> > > > > > >>>>>>>>>>>>>>> the commenter is not going to >> help/contribute a >> >> > > > > > >>>> better >> >> > > > > > >>>>> fix. >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> This is now a paramount thing for me if >> people >> >> in >> >> > > > > > >>>> this >> >> > > > > > >>>>>>> thread >> >> > > > > > >>>>>>>>>> will >> >> > > > > > >>>>>>>>>>> join >> >> > > > > > >>>>>>>>>>>>>>> the >> >> > > > > > >>>>>>>>>>>>>>> process or not. People may be not happy with >> >> some >> >> > > > > > >>>>>>>>>>> decisions/code/style, >> >> > > > > > >>>>>>>>>>>>>>> and >> >> > > > > > >>>>>>>>>>>>>>> some people are more often unhappy than >> others. >> >> > > > > > >>> More >> >> > > > > > >>>>> you >> >> > > > > > >>>>>>>>>>> contribute,- more >> >> > > > > > >>>>>>>>>>>>>>> you can decide. If you don't contribute at >> all - >> >> > > > > > >> I >> >> > > > > > >>>>> don't >> >> > > > > > >>>>>>>> care too >> >> > > > > > >>>>>>>>>>> much >> >> > > > > > >>>>>>>>>>>>>>> about just opinions, I can accept facts. To >> >> > > > > > >> provide >> >> > > > > > >>>>> facts >> >> > > > > > >>>>>>> we >> >> > > > > > >>>>>>>> need >> >> > > > > > >>>>>>>>>>> to do >> >> > > > > > >>>>>>>>>>>>>>> deep research, how can someone know if the >> test >> >> > > > > > >>>> should >> >> > > > > > >>>>> be >> >> > > > > > >>>>>>>> no-op >> >> > > > > > >>>>>>>>>> or >> >> > > > > > >>>>>>>>>>> not >> >> > > > > > >>>>>>>>>>>>>>> without deep analysis? >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> Again, if someone comes to list and provide >> just >> >> > > > > > >>>>> negative >> >> > > > > > >>>>>>>>>>> feedback, people >> >> > > > > > >>>>>>>>>>>>>>> will stop writing here. Probably no-op was >> >> > > > > > >> enabled >> >> > > > > > >>>>> without >> >> > > > > > >>>>>>>> proper >> >> > > > > > >>>>>>>>>>>>>>> discussion because of this, someone may be >> >> afraid >> >> > > > > > >>> of >> >> > > > > > >>>>>>> sharing >> >> > > > > > >>>>>>>>>> this. >> >> > > > > > >>>>>>>>>>> Result: >> >> > > > > > >>>>>>>>>>>>>>> some of us knew it only now. >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> Do you need to make Ignite quite toxic place >> to >> >> > > > > > >>> have >> >> > > > > > >>>> an >> >> > > > > > >>>>>>>>>> absolutely >> >> > > > > > >>>>>>>>>>> perfect >> >> > > > > > >>>>>>>>>>>>>>> code with just a few of arguing-resistant >> >> > > > > > >>>>> contributors? I >> >> > > > > > >>>>>>>> believe >> >> > > > > > >>>>>>>>>>> not, and >> >> > > > > > >>>>>>>>>>>>>>> you don't need to be reminded 'community >> first >> >> > > > > > >>>>> principle'. >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> ср, 5 дек. 2018 г. в 19:43, Nikolay Izhikov < >> >> > > > > > >>>>>>>> [email protected] >> >> > > > > > >>>>>>>>>>> : >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> Dmitriy. >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> I think we should avoid copy paste code >> instead >> >> > > > > > >>> of >> >> > > > > > >>>>>>> thinking >> >> > > > > > >>>>>>>>>>> about Apache >> >> > > > > > >>>>>>>>>>>>>>>> Way all the time :) >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> Anyway, I propose to return to the code! >> >> > > > > > >>>>>>>>>>>>>>>> I think we should use some kind of marker >> base >> >> > > > > > >>>> class >> >> > > > > > >>>>> for >> >> > > > > > >>>>>>> a >> >> > > > > > >>>>>>>>>> cases >> >> > > > > > >>>>>>>>>>> with >> >> > > > > > >>>>>>>>>>>>>>>> NoOpHandler. >> >> > > > > > >>>>>>>>>>>>>>>> This has several advantages, comparing with >> >> > > > > > >>> current >> >> > > > > > >>>>>>>>>>> implementation: >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> 1. No copy paste code >> >> > > > > > >>>>>>>>>>>>>>>> 2. Reduce changes. >> >> > > > > > >>>>>>>>>>>>>>>> 3. All usages of NoOpHandler can be easily >> >> > > > > > >> found >> >> > > > > > >>>>> with IDE >> >> > > > > > >>>>>>>> or >> >> > > > > > >>>>>>>>>> grep >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> search. >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> I've prepared proof of concept pull request >> to >> >> > > > > > >>>>>>> demonstrate >> >> > > > > > >>>>>>>> my >> >> > > > > > >>>>>>>>>>> approach >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> [1] >> >> > > > > > >>>>>>>>>>>>>>>> I can go further and prepare full fix. >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> What do you think? >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> [1] >> >> > > > > > >>>> https://github.com/apache/ignite/pull/5584/files >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> ср, 5 дек. 2018 г. в 18:29, Dmitriy Pavlov < >> >> > > > > > >>>>>>>> [email protected] >> >> > > > > > >>>>>>>>>>> : >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> Folks, let me explain one thing which is >> not >> >> > > > > > >>>>> related >> >> > > > > > >>>>>>>> much to >> >> > > > > > >>>>>>>>>>> fix >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> itself, >> >> > > > > > >>>>>>>>>>>>>>>>> but it is more about how we interact. If >> >> > > > > > >>> someone >> >> > > > > > >>>>> will >> >> > > > > > >>>>>>>> just >> >> > > > > > >>>>>>>>>>> come to the >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> list >> >> > > > > > >>>>>>>>>>>>>>>>> and say it is not good commit, it is a >> silly >> >> > > > > > >>>>> solution >> >> > > > > > >>>>>>>> and say >> >> > > > > > >>>>>>>>>>> to >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> others >> >> > > > > > >>>>>>>>>>>>>>>> to >> >> > > > > > >>>>>>>>>>>>>>>>> rework these patches - it is a road to >> >> > > > > > >> nowhere. >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> If someone sees the potential to make >> things >> >> > > > > > >>>>> better he >> >> > > > > > >>>>>>>> or she >> >> > > > > > >>>>>>>>>>> suggest >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> help >> >> > > > > > >>>>>>>>>>>>>>>>> (or commits patch). This is named >> do-ocracy, >> >> > > > > > >>>> those >> >> > > > > > >>>>> who >> >> > > > > > >>>>>>>> do can >> >> > > > > > >>>>>>>>>>> make a >> >> > > > > > >>>>>>>>>>>>>>>>> decision. >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> And this topic it is a perfect example of >> how >> >> > > > > > >>>>> do-ocracy >> >> > > > > > >>>>>>>>>> should >> >> > > > > > >>>>>>>>>>> (and >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> should >> >> > > > > > >>>>>>>>>>>>>>>>> not) work. We have a potentially hidden >> >> > > > > > >> problem >> >> > > > > > >>>>> (we had >> >> > > > > > >>>>>>>> it >> >> > > > > > >>>>>>>>>>> before >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> Dmitriy >> >> > > > > > >>>>>>>>>>>>>>>>> R. commit), I believe 3 or 7 tests may be >> >> > > > > > >> found >> >> > > > > > >>>>> after >> >> > > > > > >>>>>>>>>>> re-checks of >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> tests. >> >> > > > > > >>>>>>>>>>>>>>>>> Eventually, these tests will get their >> >> > > > > > >>> stop-node >> >> > > > > > >>>>>>> handler >> >> > > > > > >>>>>>>>>> after >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> revisiting >> >> > > > > > >>>>>>>>>>>>>>>>> no-op test list. >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> We have ~100 tests and several people who >> >> > > > > > >> care. >> >> > > > > > >>>>> Anton, >> >> > > > > > >>>>>>>>>> Andrew, >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> Dmitrii & >> >> > > > > > >>>>>>>>>>>>>>>>> Dmitriy, Nikolay, probably Ed, and we have >> >> > > > > > >>> 100/6 >> >> > > > > > >>>> = >> >> > > > > > >>>>> 18 >> >> > > > > > >>>>>>>> tests >> >> > > > > > >>>>>>>>>> to >> >> > > > > > >>>>>>>>>>> double >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> check >> >> > > > > > >>>>>>>>>>>>>>>>> for each contributor. We can make things >> >> > > > > > >> better >> >> > > > > > >>>> if >> >> > > > > > >>>>> we >> >> > > > > > >>>>>>> go >> >> > > > > > >>>>>>>>>>> together. And >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> this >> >> > > > > > >>>>>>>>>>>>>>>>> is how a community works. >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> If someone just come to list to criticize >> and >> >> > > > > > >>>>> enforces >> >> > > > > > >>>>>>>>>> someone >> >> > > > > > >>>>>>>>>>> else >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> to do >> >> > > > > > >>>>>>>>>>>>>>>>> all things, he or she probably don't want >> to >> >> > > > > > >>>>> improve >> >> > > > > > >>>>>>>> project >> >> > > > > > >>>>>>>>>>> code but >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> has >> >> > > > > > >>>>>>>>>>>>>>>>> other goals. >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> ср, 5 дек. 2018 г. в 18:08, Andrey >> Kuznetsov >> >> > > > > > >> < >> >> > > > > > >>>>>>>>>>> [email protected]>: >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>> As I can see from the above discussion, >> >> > > > > > >>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>> Tests in these classes check fail cases >> >> > > > > > >>> when >> >> > > > > > >>>>> we >> >> > > > > > >>>>>>>> expect >> >> > > > > > >>>>>>>>>>> critical >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> failure >> >> > > > > > >>>>>>>>>>>>>>>>>> like node stop or exception thrown >> >> > > > > > >>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>> So, this copy-n-paste-style change is >> >> > > > > > >> caused >> >> > > > > > >>> by >> >> > > > > > >>>>> the >> >> > > > > > >>>>>>>>>>> imperfect logic >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> of >> >> > > > > > >>>>>>>>>>>>>>>>>> existing tests, that should be reworked in >> >> > > > > > >>> more >> >> > > > > > >>>>>>> robust >> >> > > > > > >>>>>>>> way, >> >> > > > > > >>>>>>>>>>> e.g. >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> using >> >> > > > > > >>>>>>>>>>>>>>>>>> custom failure handlers. Dmitrii just >> >> > > > > > >>> revealed >> >> > > > > > >>>>> the >> >> > > > > > >>>>>>>> existing >> >> > > > > > >>>>>>>>>>> flaws, >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> IMO. >> >> > > > > > >>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>> ср, 5 дек. 2018 г. в 17:54, Nikolay >> >> > > > > > >> Izhikov < >> >> > > > > > >>>>>>>>>>> [email protected]>: >> >> > > > > > >>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>> Hello, Igniters. >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>> I'm agree with Anton Vinogradov. >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>> I think we should avoid commits like [1] >> >> > > > > > >>>>>>>>>>>>>>>>>>> Copy paste coding style is well known >> >> > > > > > >> anti >> >> > > > > > >>>>> pattern. >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>> Don't we have another option to do same >> >> > > > > > >> fix >> >> > > > > > >>>>> with >> >> > > > > > >>>>>>>> better >> >> > > > > > >>>>>>>>>>> styling? >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>> Accepting such patches leads to the >> >> > > > > > >> further >> >> > > > > > >>>>> tickets >> >> > > > > > >>>>>>>> to >> >> > > > > > >>>>>>>>>>> cleanup >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> mess >> >> > > > > > >>>>>>>>>>>>>>>>> that >> >> > > > > > >>>>>>>>>>>>>>>>>>> patches brings to the code base. >> >> > > > > > >>>>>>>>>>>>>>>>>>> Example of cleanup [2] >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>> It's take a significant amount of my and >> >> > > > > > >>>> Maxim >> >> > > > > > >>>>> time >> >> > > > > > >>>>>>>> to >> >> > > > > > >>>>>>>>>>> made and >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> review >> >> > > > > > >>>>>>>>>>>>>>>>>> this >> >> > > > > > >>>>>>>>>>>>>>>>>>> cleanup patch. >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>> We shouldn't accept patch with copy paste >> >> > > > > > >>>>>>>> "improvements". >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>> I really like your perfectionism >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>> It's not about perfectionism it's about >> >> > > > > > >>>> keeping >> >> > > > > > >>>>>>> code >> >> > > > > > >>>>>>>> base >> >> > > > > > >>>>>>>>>>> clean. >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>> And I'm going to rollback changes in >> >> > > > > > >> case >> >> > > > > > >>>>>>> arguments >> >> > > > > > >>>>>>>>>> will >> >> > > > > > >>>>>>>>>>> not be >> >> > > > > > >>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>> provided. >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>> +1 to rollback and rework this commit. >> >> > > > > > >>>>>>>>>>>>>>>>>>> At least, we should reduce copy paste >> >> > > > > > >> code. >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>> [1] >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>> >> >> > > > > > >>>>>>>>>> >> >> > > > > > >>>>>>>> >> >> > > > > > >>>>>>> >> >> > > > > > >>>>> >> >> > > > > > >>>> >> >> > > > > > >>> >> >> > > > > > >> >> >> > > > > > >> >> > > > > > >> >> > > > > >> >> > > > >> >> > >> >> >> https://github.com/apache/ignite/commit/b94a3c2fe3a272a31fad62b80505d16f87eab2dd >> >> > > > > > >>>>>>>>>>>>>>>>>>> [2] >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>> >> >> > > > > > >>>>>>>>>> >> >> > > > > > >>>>>>>> >> >> > > > > > >>>>>>> >> >> > > > > > >>>>> >> >> > > > > > >>>> >> >> > > > > > >>> >> >> > > > > > >> >> >> > > > > > >> >> > > > > > >> >> > > > > >> >> > > > >> >> > >> >> >> https://github.com/apache/ignite/commit/eb8038f65285559c5424eba2882b0de0583ea7af >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>> ср, 5 дек. 2018 г. в 17:28, Anton >> >> > > > > > >>> Vinogradov >> >> > > > > > >>>> < >> >> > > > > > >>>>>>>>>>> [email protected]>: >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>> Andrey, >> >> > > > > > >>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>> But why should we make all things >> >> > > > > > >>>> perfect >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>> in a single fix? >> >> > > > > > >>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>> As I said, I'm ok in case someone ready >> >> > > > > > >>> to >> >> > > > > > >>>>>>>> continue :) >> >> > > > > > >>>>>>>>>>>>>>>>>>>> But, we should avoid such >> >> > > > > > >>> over-copy-pasted >> >> > > > > > >>>>>>> commits >> >> > > > > > >>>>>>>> in >> >> > > > > > >>>>>>>>>> the >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> future. >> >> > > > > > >>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>> On Wed, Dec 5, 2018 at 5:13 PM Andrey >> >> > > > > > >>>>> Mashenkov < >> >> > > > > > >>>>>>>>>>>>>>>>>>>> [email protected]> >> >> > > > > > >>>>>>>>>>>>>>>>>>>> wrote: >> >> > > > > > >>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>> Dmitry, >> >> > > > > > >>>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>> Do we have TC run results for the PR >> >> > > > > > >>>> before >> >> > > > > > >>>>>>>> massive >> >> > > > > > >>>>>>>>>>> failure >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> handler >> >> > > > > > >>>>>>>>>>>>>>>>>>>>> fallbacks were added? >> >> > > > > > >>>>>>>>>>>>>>>>>>>>> Let's create a ticket to investigate >> >> > > > > > >>>>>>> possibility >> >> > > > > > >>>>>>>> of >> >> > > > > > >>>>>>>>>>> using any >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>> meaningful >> >> > > > > > >>>>>>>>>>>>>>>>>>>>> failure handler for such tests with >> >> > > > > > >> TC >> >> > > > > > >>>>> report >> >> > > > > > >>>>>>>>>> attached. >> >> > > > > > >>>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>> On Wed, Dec 5, 2018 at 4:41 PM Anton >> >> > > > > > >>>>>>> Vinogradov < >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> [email protected]> >> >> > > > > > >>>>>>>>>>>>>>>>>> wrote: >> >> > > > > > >>>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>> Dmitriy, >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>> It's ok in case someone ready to do >> >> > > > > > >>>> this >> >> > > > > > >>>>> (get >> >> > > > > > >>>>>>>> rid >> >> > > > > > >>>>>>>>>> of >> >> > > > > > >>>>>>>>>>> all >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> no-op >> >> > > > > > >>>>>>>>>>>>>>>> or >> >> > > > > > >>>>>>>>>>>>>>>>>>>> explain >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>> why it's a better choice). >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>> Explicit confirmation required. >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>> Otherwise, only rollback is an >> >> > > > > > >>> option. >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>> On Wed, Dec 5, 2018 at 4:29 PM >> >> > > > > > >>> Dmitriy >> >> > > > > > >>>>>>> Pavlov < >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> [email protected]> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>> wrote: >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> Anton, if you care enough here >> >> > > > > > >> will >> >> > > > > > >>>>> you try >> >> > > > > > >>>>>>>> to >> >> > > > > > >>>>>>>>>>> research a >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> couple >> >> > > > > > >>>>>>>>>>>>>>>>>> of >> >> > > > > > >>>>>>>>>>>>>>>>>>>>> these >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> tests? Or you are asking others >> >> > > > > > >> to >> >> > > > > > >>> do >> >> > > > > > >>>>>>> things >> >> > > > > > >>>>>>>> for >> >> > > > > > >>>>>>>>>>> you, >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> aren't >> >> > > > > > >>>>>>>>>>>>>>>>> you? >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> I like idea from Andrew to create >> >> > > > > > >>>>> ticket >> >> > > > > > >>>>>>> and >> >> > > > > > >>>>>>>>>> check >> >> > > > > > >>>>>>>>>>> these >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> test >> >> > > > > > >>>>>>>>>>>>>>>>> to >> >> > > > > > >>>>>>>>>>>>>>>>>>> keep >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> moving towards 0....10 tests with >> >> > > > > > >>>>> noop. It >> >> > > > > > >>>>>>> is >> >> > > > > > >>>>>>>>>> easy >> >> > > > > > >>>>>>>>>>> to >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> locate >> >> > > > > > >>>>>>>>>>>>>>>>>> these >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> overridden method now. >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> So threat this change as >> >> > > > > > >>> contributed >> >> > > > > > >>>>>>>> mechanism >> >> > > > > > >>>>>>>>>> for >> >> > > > > > >>>>>>>>>>> failing >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> tests. >> >> > > > > > >>>>>>>>>>>>>>>>>>> Is >> >> > > > > > >>>>>>>>>>>>>>>>>>>> it >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>> Ok >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> for you? >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> ср, 5 дек. 2018 г., 15:59 Anton >> >> > > > > > >>>>> Vinogradov >> >> > > > > > >>>>>>> < >> >> > > > > > >>>>>>>>>>> [email protected] >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> : >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>> I didn't get. What is the >> >> > > > > > >>>>> problem in >> >> > > > > > >>>>>>>> saving >> >> > > > > > >>>>>>>>>>> No-Op for >> >> > > > > > >>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>> several >> >> > > > > > >>>>>>>>>>>>>>>>>>>>> tests? >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> Why >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>> should we keep No-Op for >> >> > > > > > >> all? >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> Several (less than 10) is ok to >> >> > > > > > >>> me >> >> > > > > > >>>>> with >> >> > > > > > >>>>>>> the >> >> > > > > > >>>>>>>>>>> proper >> >> > > > > > >>>>>>>>> >> >> >> > > -- >
