Default failure handler was changed for tests
Locked
1234
1234
Re: Default failure handler was changed for tests
 
|
Reverted.
https://issues.apache.org/jira/browse/IGNITE-8227 reopened пн, 10 дек. 2018 г. в 16:23, Dmitriy Pavlov <[hidden email]>: > 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 <[hidden email]>: > >> 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 <[hidden email]> >> wrote: >> >> > Agree, it is reasonable to revert. >> > пт, 7 дек. 2018 г. в 18:44, Dmitriy Pavlov <[hidden email]>: >> > > >> > > 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 <[hidden email]>: >> > > >> > > > 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 <[hidden email]> >> 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 < >> [hidden email]> >> > > > > 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 < >> > [hidden email] >> > > > > >> > > > > > 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 [hidden email]: >> > > > > > >> >> > > > > > >>> 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 <[hidden email] >> >: >> > > > > > >>> >> > > > > > >>>> 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 Павлухин Иван < >> > [hidden email] >> > > > > >> > > > > > >>> 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 < >> > > > [hidden email] >> > > > > >: >> > > > > > >>>>>> >> > > > > > >>>>>> 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 [hidden email] >> : >> > > > > > >>>>>> >> > > > > > >>>>>>> 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 Павлухин Иван < >> > > > > > >> [hidden email] >> > > > > > >>>> >> > > > > > >>>>> 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 < >> > > > > > >> [hidden email] >> > > > > > >>>> : >> > > > > > >>>>>>>>> >> > > > > > >>>>>>>>> Dmitriy Ryabov, Dmitriy Pavlov, sorry. >> > > > > > >>>>>>>>> >> > > > > > >>>>>>>>> Of course it should be "NOT to blame author". >> > > > > > >>>>>>>>> >> > > > > > >>>>>>>>> Sorry, one more time. >> > > > > > >>>>>>>>> >> > > > > > >>>>>>>>> чт, 6 дек. 2018 г., 10:40 Dmitriy Pavlov >> > [hidden email]: >> > > > > > >>>>>>>>> >> > > > > > >>>>>>>>>> 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 < >> > > > > > >>>> [hidden email] >> > > > > > >>>>>> : >> > > > > > >>>>>>>>>> >> > > > > > >>>>>>>>>>> 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 < >> > > > > > >>>>> [hidden email] >> > > > > > >>>>>>>> : >> > > > > > >>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>> 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 < >> > > > > > >>>>>>> [hidden email] >> > > > > > >>>>>>>>> : >> > > > > > >>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> 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 < >> > > > > > >>>>>>> [hidden email] >> > > > > > >>>>>>>>> : >> > > > > > >>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> 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 < >> > > > > > >>>>>>>> [hidden email] >> > > > > > >>>>>>>>>>> : >> > > > > > >>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>> 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 < >> > > > > > >>>>>>>> [hidden email] >> > > > > > >>>>>>>>>>> : >> > > > > > >>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>> 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 >> > > > > > >> < >> > > > > > >>>>>>>>>>> [hidden email]>: >> > > > > > >>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>> 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 < >> > > > > > >>>>>>>>>>> [hidden email]>: >> > > > > > >>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>> 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 >> > > > > > >>>> < >> > > > > > >>>>>>>>>>> [hidden email]>: >> > > > > > >>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>>> 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 < >> > > > > > >>>>>>>>>>>>>>>>>>>> [hidden email]> >> > > > > > >>>>>>>>>>>>>>>>>>>> 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 < >> > > > > > >>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> [hidden email]> >> > > > > > >>>>>>>>>>>>>>>>>> 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 < >> > > > > > >>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>> [hidden email]> >> > > > > > >>>>>>>>>>>>>>>>>>>>> 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 >> > > > > > >>>>>>> < >> > > > > > >>>>>>>>>>> [hidden email] >> > > > > > >>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>> : >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>> 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 >> > > > > > >>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>> explanation >> > > > > > >>>>>>>>>>>>>>>>>>> why >> > > > > > >>>>>>>>>>>>>>>>>>>>>> tests >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> fail and why no-op is a better >> > > > > > >>>>> choice. >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> 100+++ copy-pasted no-op >> > > > > > >> handlers >> > > > > > >>>>> are not >> > > > > > >>>>>>>> ok! >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>> I don't ask you to re-do >> > > > > > >> this >> > > > > > >>>>> change, >> > > > > > >>>>>>>> I ask >> > > > > > >>>>>>>>>>> to >> > > > > > >>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>> demonstrate >> > > > > > >>>>>>>>>>>>>>>>>> any >> > > > > > >>>>>>>>>>>>>>>>>>>>>> better >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>> approach for tests which >> > > > > > >>>>>>> intentionally >> > > > > > >>>>>>>>>>> activate >> > > > > > >>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> failure >> > > > > > >>>>>>>>>>>>>>>>>>> handler. >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> You asking me to provide >> > > > > > >> approach >> > > > > > >>>>> without >> > > > > > >>>>>>>>>>> explanation >> > > > > > >>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> why >> > > > > > >>>>>>>>>>>>>>>>> tests >> > > > > > >>>>>>>>>>>>>>>>>>>> fail >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> without no-op handler? >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> My approach is to rollback this >> > > > > > >>>> fix, >> > > > > > >>>>>>>> reopen the >> > > > > > >>>>>>>>>>> issue >> > > > > > >>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> and >> > > > > > >>>>>>>>>>>>>>>>> make >> > > > > > >>>>>>>>>>>>>>>>>>>>>> everything >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> properly. >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> Make a proper investigation >> > > > > > >>> first. >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> Finally, let's stop this game. >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> We have to discuss the reasons >> > > > > > >>> why >> > > > > > >>>>> tests >> > > > > > >>>>>>>> fail. >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> In case no-one checked "why" >> > > > > > >>> before >> > > > > > >>>>> the >> > > > > > >>>>>>>> fix was >> > > > > > >>>>>>>>>>> merged >> > > > > > >>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> we >> > > > > > >>>>>>>>>>>>>>>>> will >> > > > > > >>>>>>>>>>>>>>>>>> be >> > > > > > >>>>>>>>>>>>>>>>>>>>> able >> > > > > > >>>>>>>>>>>>>>>>>>>>>> to >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> start doing this after >> > > > > > >> rollback. >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> On Wed, Dec 5, 2018 at 3:49 PM >> > > > > > >>>> Eduard >> > > > > > >>>>>>>>>> Shangareev >> > > > > > >>>>>>>>>>> < >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> [hidden email]> >> > > > > > >>>> wrote: >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>> Guys, >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>> I didn't get. What is the >> > > > > > >>> problem >> > > > > > >>>>> in >> > > > > > >>>>>>>> saving >> > > > > > >>>>>>>>>>> No-Op for >> > > > > > >>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>> several >> > > > > > >>>>>>>>>>>>>>>>>>>>> tests? >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> Why >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>> should we keep No-Op for all? >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Dec 5, 2018 at 3:20 >> > > > > > >> PM >> > > > > > >>>>> Павлухин >> > > > > > >>>>>>>> Иван >> > > > > > >>>>>>>>>> < >> > > > > > >>>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>>> [hidden email]> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> wrote: >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>> Anton, >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>> Yes I meant that patch. >> > > > > > >> And I >> > > > > > >>>>> would >> > > > > > >>>>>>>> like to >> > > > > > >>>>>>>>>>> respell >> > > > > > >>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> a >> > > > > > >>>>>>>>>>>>>>>>> name >> > > > > > >>>>>>>>>>>>>>>>>> > > |
Re: Default failure handler was changed for tests
|
|
Ilya, can you check your test on current implementation [1]?
[1] https://github.com/apache/ignite/pull/5662 10 дек. 2018 г. 17:10 пользователь "Dmitriy Pavlov" <[hidden email]> написал: Reverted. https://issues.apache.org/jira/browse/IGNITE-8227 reopened пн, 10 дек. 2018 г. в 16:23, Dmitriy Pavlov <[hidden email]>: > 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 <[hidden email]>: > >> 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 <[hidden email]> >> wrote: >> >> > Agree, it is reasonable to revert. >> > пт, 7 дек. 2018 г. в 18:44, Dmitriy Pavlov <[hidden email]>: >> > > >> > > 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 <[hidden email]>: >> > > >> > > > 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 >> > > > >> > > > >> > >> >> > > > 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 <[hidden email]> >> 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 < >> [hidden email]> >> > > > > 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 >> > > > > > 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 < >> > [hidden email] >> > > > > >> > > > > > 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 [hidden email]: >> > > > > > >> >> > > > > > >>> 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 <[hidden email] >> >: >> > > > > > >>> >> > > > > > >>>> 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 Павлухин Иван < >> > [hidden email] >> > > > > >> > > > > > >>> 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 < >> > > > [hidden email] >> > > > > >: >> > > > > > >>>>>> >> > > > > > >>>>>> 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 [hidden email] >> : >> > > > > > >>>>>> >> > > > > > >>>>>>> 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 Павлухин Иван < >> > > > > > >> [hidden email] >> > > > > > >>>> >> > > > > > >>>>> 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 >> > ~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 < >> > > > > > >> [hidden email] >> > > > > > >>>> : >> > > > > > >>>>>>>>> >> > > > > > >>>>>>>>> Dmitriy Ryabov, Dmitriy Pavlov, sorry. >> > > > > > >>>>>>>>> >> > > > > > >>>>>>>>> Of course it should be "NOT to blame author". >> > > > > > >>>>>>>>> >> > > > > > >>>>>>>>> Sorry, one more time. >> > > > > > >>>>>>>>> >> > > > > > >>>>>>>>> чт, 6 дек. 2018 г., 10:40 Dmitriy Pavlov >> > [hidden email]: >> > > > > > >>>>>>>>> >> > > > > > >>>>>>>>>> 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 < >> > > > > > >>>> [hidden email] >> > > > > > >>>>>> : >> > > > > > >>>>>>>>>> >> > > > > > >>>>>>>>>>> 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 >> > > > > > >> 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 >> > > > > > >> 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 >> > > > > > >>> 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 < >> > > > > > >>>>> [hidden email] >> > > > > > >>>>>>>> : >> > > > > > >>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>> 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 < >> > > > > > >>>>>>> [hidden email] >> > > > > > >>>>>>>>> : >> > > > > > >>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> 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 < >> > > > > > >>>>>>> [hidden email] >> > > > > > >>>>>>>>> : >> > > > > > >>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> 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 - >> > > > > > >> 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 >> > > > > > >>> are >> > > > > > >>>>>>> allowed >> > > > > > >>>>>>>> to >> > > > > > >>>>>>>>>>> have with >> > > > > > >>>>>>>>>>>>>>> no-op: please visit this page >> > > > > > >>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>> >> > > > > > >>>>>>>>>> >> > > > > > >>>>>>>> >> > > > > > >>>>>>> >> > > > > > >>>>> >> > > > > > >>>> >> > > > > > >>> >> > > > > > >> >> > > > > > >> > > > > > >> > > > > >> > > > >> > >> >> > > > > > >>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> 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 >> > > > > > >> 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 >> > > > > > >>>>> 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 < >> > > > > > >>>>>>>> [hidden email] >> > > > > > >>>>>>>>>>> : >> > > > > > >>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>> Dmitriy. >> > > > > > >>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>> I think we should avoid copy paste code >> > > > > > >>> 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 < >> > > > > > >>>>>>>> [hidden email] >> > > > > > >>>>>>>>>>> : >> > > > > > >>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>> 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 >> > > > > > >> < >> > > > > > >>>>>>>>>>> [hidden email]>: >> > > > > > >>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>> 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 < >> > > > > > >>>>>>>>>>> [hidden email]>: >> > > > > > >>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>> 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] >> > > > > > >>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>> >> > > > > > >>>>>>>>>> >> > > > > > >>>>>>>> >> > > > > > >>>>>>> >> > > > > > >>>>> >> > > > > > >>>> >> > > > > > >>> >> > > > > > >> >> > > > > > >> > > > > > >> > > > > >> > > > >> > >> >> > > > > > >>>>>>>>>>>>>>>>>>> [2] >> > > > > > >>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>> >> > > > > > >>>>>>>>>> >> > > > > > >>>>>>>> >> > > > > > >>>>>>> >> > > > > > >>>>> >> > > > > > >>>> >> > > > > > >>> >> > > > > > >> >> > > > > > >> > > > > > >> > > > > >> > > > >> > >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>> ср, 5 дек. 2018 г. в 17:28, Anton >> > > > > > >>> Vinogradov >> > > > > > >>>> < >> > > > > > >>>>>>>>>>> [hidden email]>: >> > > > > > >>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>>> 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 < >> > > > > > >>>>>>>>>>>>>>>>>>>> [hidden email]> >> > > > > > >>>>>>>>>>>>>>>>>>>> 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 < >> > > > > > >>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>> [hidden email]> >> > > > > > >>>>>>>>>>>>>>>>>> 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 < >> > > > > > >>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>> [hidden email]> >> > > > > > >>>>>>>>>>>>>>>>>>>>> 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 >> > > > > > >>>>>>> < >> > > > > > >>>>>>>>>>> [hidden email] >> > > > > > >>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>> : >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>> 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 >> > > > > > >>>>>>>>> |
Re: Default failure handler was changed for tests
|
|
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 <[hidden email]> 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" <[hidden email]> > написал: > > Reverted. > > https://issues.apache.org/jira/browse/IGNITE-8227 reopened > > пн, 10 дек. 2018 г. в 16:23, Dmitriy Pavlov <[hidden email]>: > > > > 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 <[hidden email]>: > > > >> 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 <[hidden email]> > >> wrote: > >> > >> > Agree, it is reasonable to revert. > >> > пт, 7 дек. 2018 г. в 18:44, Dmitriy Pavlov <[hidden email]>: > >> > > > >> > > 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 <[hidden email]>: > >> > > > >> > > > 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 <[hidden email]> > >> 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 < > >> [hidden email]> > >> > > > > 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 < > >> > [hidden email] > >> > > > > > >> > > > > > 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 > [hidden email]: > >> > > > > > >> > >> > > > > > >>> 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 < > [hidden email] > >> >: > >> > > > > > >>> > >> > > > > > >>>> 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 Павлухин Иван < > >> > [hidden email] > >> > > > > > >> > > > > > >>> 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 < > >> > > > [hidden email] > >> > > > > >: > >> > > > > > >>>>>> > >> > > > > > >>>>>> 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 > [hidden email] > >> : > >> > > > > > >>>>>> > >> > > > > > >>>>>>> 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 Павлухин Иван < > >> > > > > > >> [hidden email] > >> > > > > > >>>> > >> > > > > > >>>>> 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 < > >> > > > > > >> [hidden email] > >> > > > > > >>>> : > >> > > > > > >>>>>>>>> > >> > > > > > >>>>>>>>> Dmitriy Ryabov, Dmitriy Pavlov, sorry. > >> > > > > > >>>>>>>>> > >> > > > > > >>>>>>>>> Of course it should be "NOT to blame author". > >> > > > > > >>>>>>>>> > >> > > > > > >>>>>>>>> Sorry, one more time. > >> > > > > > >>>>>>>>> > >> > > > > > >>>>>>>>> чт, 6 дек. 2018 г., 10:40 Dmitriy Pavlov > >> > [hidden email]: > >> > > > > > >>>>>>>>> > >> > > > > > >>>>>>>>>> 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 < > >> > > > > > >>>> [hidden email] > >> > > > > > >>>>>> : > >> > > > > > >>>>>>>>>> > >> > > > > > >>>>>>>>>>> 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 < > >> > > > > > >>>>> [hidden email] > >> > > > > > >>>>>>>> : > >> > > > > > >>>>>>>>>>>>> > >> > > > > > >>>>>>>>>>>>> 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 < > >> > > > > > >>>>>>> [hidden email] > >> > > > > > >>>>>>>>> : > >> > > > > > >>>>>>>>>>>>> > >> > > > > > >>>>>>>>>>>>>>> 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 < > >> > > > > > >>>>>>> [hidden email] > >> > > > > > >>>>>>>>> : > >> > > > > > >>>>>>>>>>>>>> > >> > > > > > >>>>>>>>>>>>>>> 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 < > >> > > > > > >>>>>>>> [hidden email] > >> > > > > > >>>>>>>>>>> : > >> > > > > > >>>>>>>>>>>>>>> > >> > > > > > >>>>>>>>>>>>>>>> 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 < > >> > > > > > >>>>>>>> [hidden email] > >> > > > > > >>>>>>>>>>> : > >> > > > > > >>>>>>>>>>>>>>>> > >> > > > > > >>>>>>>>>>>>>>>>> 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 > >> > > > > > >> < > >> > > > > > >>>>>>>>>>> [hidden email]>: > >> > > > > > >>>>>>>>>>>>>>>>> > >> > > > > > >>>>>>>>>>>>>>>>>> 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 < > >> > > > > > >>>>>>>>>>> [hidden email]>: > >> > > > > > >>>>>>>>>>>>>>>>>> > >> > > > > > >>>>>>>>>>>>>>>>>>> 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 > >> > > > > > >>>> < > >> > > > > > >>>>>>>>>>> [hidden email]>: > >> > > > > > >>>>>>>>>>>>>>>>>>> > >> > > > > > >>>>>>>>>>>>>>>>>>>> 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 < > >> > > > > > >>>>>>>>>>>>>>>>>>>> [hidden email]> > >> > > > > > >>>>>>>>>>>>>>>>>>>> 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 < > >> > > > > > >>>>>>>>>>>>>>> > >> > > > > > >>>>>>>>>>>>>>> [hidden email]> > >> > > > > > >>>>>>>>>>>>>>>>>> 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 < > >> > > > > > >>>>>>>>>>>>>>>>> > >> > > > > > >>>>>>>>>>>>>>>>> [hidden email]> > >> > > > > > >>>>>>>>>>>>>>>>>>>>> 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 > >> > > > > > >>>>>>> < > >> > > > > > >>>>>>>>>>> [hidden email] > >> > > > > > >>>>>>>>>>>>>>>> > >> > > > > > >>>>>>>>>>>>>>>> : > >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> > >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>> 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 > >> > > > > > >>>>>>>>> > > > -- Best regards, Ilya |
Re: Default failure handler was changed for tests
|
|
Yes, currently handler handles exception in the same way as
'GridAbstractTest', so, it will work from any thread. пт, 11 янв. 2019 г., 15:18 Ilya Lantukh [hidden email]: > 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 <[hidden email]> > 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" <[hidden email]> >> написал: >> >> Reverted. >> >> https://issues.apache.org/jira/browse/IGNITE-8227 reopened >> >> пн, 10 дек. 2018 г. в 16:23, Dmitriy Pavlov <[hidden email]>: >> >> >> > 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 <[hidden email]>: >> > >> >> 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 <[hidden email] >> > >> >> wrote: >> >> >> >> > Agree, it is reasonable to revert. >> >> > пт, 7 дек. 2018 г. в 18:44, Dmitriy Pavlov <[hidden email]>: >> >> > > >> >> > > 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 <[hidden email]>: >> >> > > >> >> > > > 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 <[hidden email]> >> >> 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 < >> >> [hidden email]> >> >> > > > > 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 < >> >> > [hidden email] >> >> > > > > >> >> > > > > > 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 >> [hidden email]: >> >> > > > > > >> >> >> > > > > > >>> 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 < >> [hidden email] >> >> >: >> >> > > > > > >>> >> >> > > > > > >>>> 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 Павлухин Иван < >> >> > [hidden email] >> >> > > > > >> >> > > > > > >>> 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 < >> >> > > > [hidden email] >> >> > > > > >: >> >> > > > > > >>>>>> >> >> > > > > > >>>>>> 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 >> [hidden email] >> >> : >> >> > > > > > >>>>>> >> >> > > > > > >>>>>>> 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 Павлухин Иван < >> >> > > > > > >> [hidden email] >> >> > > > > > >>>> >> >> > > > > > >>>>> 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 < >> >> > > > > > >> [hidden email] >> >> > > > > > >>>> : >> >> > > > > > >>>>>>>>> >> >> > > > > > >>>>>>>>> Dmitriy Ryabov, Dmitriy Pavlov, sorry. >> >> > > > > > >>>>>>>>> >> >> > > > > > >>>>>>>>> Of course it should be "NOT to blame author". >> >> > > > > > >>>>>>>>> >> >> > > > > > >>>>>>>>> Sorry, one more time. >> >> > > > > > >>>>>>>>> >> >> > > > > > >>>>>>>>> чт, 6 дек. 2018 г., 10:40 Dmitriy Pavlov >> >> > [hidden email]: >> >> > > > > > >>>>>>>>> >> >> > > > > > >>>>>>>>>> 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 < >> >> > > > > > >>>> [hidden email] >> >> > > > > > >>>>>> : >> >> > > > > > >>>>>>>>>> >> >> > > > > > >>>>>>>>>>> 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 < >> >> > > > > > >>>>> [hidden email] >> >> > > > > > >>>>>>>> : >> >> > > > > > >>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>> 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 < >> >> > > > > > >>>>>>> [hidden email] >> >> > > > > > >>>>>>>>> : >> >> > > > > > >>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> 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 < >> >> > > > > > >>>>>>> [hidden email] >> >> > > > > > >>>>>>>>> : >> >> > > > > > >>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> 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 < >> >> > > > > > >>>>>>>> [hidden email] >> >> > > > > > >>>>>>>>>>> : >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> 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 < >> >> > > > > > >>>>>>>> [hidden email] >> >> > > > > > >>>>>>>>>>> : >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> 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 >> >> > > > > > >> < >> >> > > > > > >>>>>>>>>>> [hidden email]>: >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>> 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 < >> >> > > > > > >>>>>>>>>>> [hidden email]>: >> >> > > > > > >>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>> 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 >> >> > > > > > >>>> < >> >> > > > > > >>>>>>>>>>> [hidden email]>: >> >> > > > > > >>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>> 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 < >> >> > > > > > >>>>>>>>>>>>>>>>>>>> [hidden email]> >> >> > > > > > >>>>>>>>>>>>>>>>>>>> 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 < >> >> > > > > > >>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>> [hidden email]> >> >> > > > > > >>>>>>>>>>>>>>>>>> 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 < >> >> > > > > > >>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>> [hidden email]> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>> 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 >> >> > > > > > >>>>>>> < >> >> > > > > > >>>>>>>>>>> [hidden email] >> >> > > > > > >>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>> : >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>> >> >> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>> 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 >> >> > > > > > >>>>>>>>> >> >> >> > > -- > |
«
Return to Apache Ignite Developers - Legacy Mail Archive
|
1 view|%1 views
| Free forum by Nabble | Edit this page |