Anton,
The change looks very questionable. We cannot be adding configuration
validators for every piece of Ignite API. What is it you are trying to
achieve?
D.
On Mon, May 21, 2018 at 7:22 AM, Anton Vinogradov <a...@apache.org> wrote:
> Yakov, thank's for deep check.
>
> >> I think that we should think about some other solution instead of
> altering
> >> event sub-system.
>
> Thank's to your comments now I see that solution is not perfect.
>
> How about to create
>
> interface TransactionsValidator {
> boolean validate(IgniteTransactions tx){
> ...
> }
> }
>
> and add it's setter to IgniteConfiguration?
>
> icfg.setTransactionsValidator(new CustomTransactionsValidator(...));
>
> In that case we'll gain easy and proper solution allows to check
> transaction configuration before real tx creation.
>
> It will be necessary to add some getters to IgniteTransactions:
> - label()
> - timeout()
> - concurrency() (optional)
> - isolation() (optional)
> - txSize() (optional)
>
> Thoughts?
>
> пн, 21 мая 2018 г. в 16:31, Yakov Zhdanov <yzhda...@apache.org>:
>
> > Ok, then it probably might have been created before this PR. Anyway, I
> > would not bother too much about pt 3.
> >
> > --Yakov
> >
> > 2018-05-21 16:15 GMT+03:00 Dmitry Pavlov <dpavlov....@gmail.com>:
> >
> > > Hi Yakov,
> > >
> > > I've checked this code and IgniteCacheTestSuite6 includes TxLabelTest,
> so
> > > point 3 can be considered as solved.
> > >
> > > Sincerely,
> > > Dmitriy Pavlov
> > >
> > > пн, 21 мая 2018 г. в 16:05, Yakov Zhdanov <yzhda...@apache.org>:
> > >
> > > > Anton, I have objections. Please do not merge unless we agree on
> > details.
> > > >
> > > > 1. You use internal API in public event, i.e. you cannot have user
> > > > accessing to IgniteInternalTx instance through TxEvent.
> > > > 2. Throwing runtime errors from listener is not documented and I
> doubt
> > if
> > > > it can be fully supported in the pattern you use in TxLabelTest.
> After
> > > > looking at the mentioned test user may think that throwing runtime
> > error
> > > > when notified on new node join may prohibit new node joining which is
> > not
> > > > true. Do you have any example in Ignite when throwing exception from
> > > > listener is valid and documented.
> > > > 3. TxLabelTest is not included into any suite.
> > > > 4. EVT_TX_STARTED - does not seems to be a proper and descriptive
> name
> > > >
> > > > I think that we should think about some other solution instead of
> > > altering
> > > > event sub-system.
> > > >
> > > > Also I want to ask everyone in community to request explicit approval
> > > from
> > > > community members before changing anything in transactional logic.
> > > >
> > > > Thanks!
> > > >
> > > > --Yakov
> > > >
> > >
> >
>
