ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitriy Pavlov <dpav...@apache.org>
Subject Re: Default failure handler was changed for tests
Date Mon, 10 Dec 2018 14:10:15 GMT
Reverted.

https://issues.apache.org/jira/browse/IGNITE-8227 reopened

пн, 10 дек. 2018 г. в 16:23, Dmitriy Pavlov <dpavlov@apache.org>:

> 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 <av@apache.org>:
>
>> 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 <somefireone@gmail.com>
>> wrote:
>>
>> > Agree, it is reasonable to revert.
>> > пт, 7 дек. 2018 г. в 18:44, Dmitriy Pavlov <dpavlov@apache.org>:
>> > >
>> > > 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 <ilantukh@gridgain.com>:
>> > >
>> > > > 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 <av@apache.org>
>> 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 <
>> nizhikov@apache.org>
>> > > > > 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 <
>> > somefireone@gmail.com
>> > > > >
>> > > > > > 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 dpavlov@apache.org:
>> > > > > > >>
>> > > > > > >>> 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 <av@apache.org
>> >:
>> > > > > > >>>
>> > > > > > >>>> 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 Павлухин Иван <
>> > vololo100@gmail.com
>> > > > >
>> > > > > > >>> 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 <
>> > > > somefireone@gmail.com
>> > > > > >:
>> > > > > > >>>>>>
>> > > > > > >>>>>> 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 av@apache.org
>> :
>> > > > > > >>>>>>
>> > > > > > >>>>>>> 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 Павлухин Иван <
>> > > > > > >> vololo100@gmail.com
>> > > > > > >>>>
>> > > > > > >>>>> 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 <
>> > > > > > >> nizhikov@apache.org
>> > > > > > >>>> :
>> > > > > > >>>>>>>>>
>> > > > > > >>>>>>>>> Dmitriy Ryabov, Dmitriy Pavlov, sorry.
>> > > > > > >>>>>>>>>
>> > > > > > >>>>>>>>> Of course it should be "NOT to blame author".
>> > > > > > >>>>>>>>>
>> > > > > > >>>>>>>>> Sorry, one more time.
>> > > > > > >>>>>>>>>
>> > > > > > >>>>>>>>> чт, 6 дек. 2018 г., 10:40 Dmitriy Pavlov
>> > dpavlov@apache.org:
>> > > > > > >>>>>>>>>
>> > > > > > >>>>>>>>>> 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 <
>> > > > > > >>>> nizhikov@apache.org
>> > > > > > >>>>>> :
>> > > > > > >>>>>>>>>>
>> > > > > > >>>>>>>>>>> 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 <
>> > > > > > >>>>> nizhikov@apache.org
>> > > > > > >>>>>>>> :
>> > > > > > >>>>>>>>>>>>>
>> > > > > > >>>>>>>>>>>>> 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 <
>> > > > > > >>>>>>> nizhikov@apache.org
>> > > > > > >>>>>>>>> :
>> > > > > > >>>>>>>>>>>>>
>> > > > > > >>>>>>>>>>>>>>> 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 <
>> > > > > > >>>>>>> dpavlov@apache.org
>> > > > > > >>>>>>>>> :
>> > > > > > >>>>>>>>>>>>>>
>> > > > > > >>>>>>>>>>>>>>> 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 <
>> > > > > > >>>>>>>> nizhikov@apache.org
>> > > > > > >>>>>>>>>>> :
>> > > > > > >>>>>>>>>>>>>>>
>> > > > > > >>>>>>>>>>>>>>>> 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 <
>> > > > > > >>>>>>>> dpavlov@apache.org
>> > > > > > >>>>>>>>>>> :
>> > > > > > >>>>>>>>>>>>>>>>
>> > > > > > >>>>>>>>>>>>>>>>> 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
>> > > > > > >> <
>> > > > > > >>>>>>>>>>> stkuzma@gmail.com>:
>> > > > > > >>>>>>>>>>>>>>>>>
>> > > > > > >>>>>>>>>>>>>>>>>> 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 <
>> > > > > > >>>>>>>>>>> nizhikov@apache.org>:
>> > > > > > >>>>>>>>>>>>>>>>>>
>> > > > > > >>>>>>>>>>>>>>>>>>> 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
>> > > > > > >>>> <
>> > > > > > >>>>>>>>>>> av@apache.org>:
>> > > > > > >>>>>>>>>>>>>>>>>>>
>> > > > > > >>>>>>>>>>>>>>>>>>>> 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 <
>> > > > > > >>>>>>>>>>>>>>>>>>>> andrey.mashenkov@gmail.com>
>> > > > > > >>>>>>>>>>>>>>>>>>>> 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 <
>> > > > > > >>>>>>>>>>>>>>>
>> > > > > > >>>>>>>>>>>>>>> av@apache.org>
>> > > > > > >>>>>>>>>>>>>>>>>> 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 <
>> > > > > > >>>>>>>>>>>>>>>>>
>> > > > > > >>>>>>>>>>>>>>>>> dpavlov@apache.org>
>> > > > > > >>>>>>>>>>>>>>>>>>>>> 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
>> > > > > > >>>>>>> <
>> > > > > > >>>>>>>>>>> av@apache.org
>> > > > > > >>>>>>>>>>>>>>>>
>> > > > > > >>>>>>>>>>>>>>>> :
>> > > > > > >>>>>>>>>>>>>>>>>>>>>>>
>> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>> 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
>> > > > > > >>>>>>>>>>> <
>> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> eduard.shangareev@gmail.com>
>> > > > > > >>>> 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
>> > > > > > >>>>> Павлухин
>> > > > > > >>>>>>>> Иван
>> > > > > > >>>>>>>>>> <
>> > > > > > >>>>>>>>>>>>>>>>>>>>
>> > > > > > >>>>>>>>>>>>>>>>>>>> vololo100@gmail.com>
>> > > > > > >>>>>>>>>>>>>>>>>>>>>>>> wrote:
>> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>
>> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>> Anton,
>> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>>
>> > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>> Yes I meant that patch.
>> > > > > > >> And I
>> > > > > > >>>>> would
>> > > > > > >>>>>>>> like to
>> > > > > > >>>>>>>>>>> respell
>> > > > > > >>>>>>>>>>>>>>>
>> > > > > > >>>>>>>>>>>>>>> a
>> > > > > > >>>>>>>>>>>>>>>>> name
>> > > > > > >>>>>>>>>>>>>>>>>>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message