You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Denis Garus <ga...@gmail.com> on 2019/09/24 10:54:59 UTC
JavaDoc for Event's subjectId methods
Hello, Igniters!
Some events contain the subjectId method, for example, TaskEvent#subjectId.
The JavaDoc for this method is:
"Gets security subject ID initiated this task event, if available.
This property is not available for GridEventType#EVT_TASK_SESSION_ATTR_SET
task event.
Subject ID will be set either to node ID or client ID initiated task
execution."
I think It's wrong. The main point is a subject id doesn't have any sense
if IgniteSecurity is disabled.
However, if IgniteSecurity is enabled, the method must return the subject
id from the current security context.
Thus, the description (and behavior) of the method should be the following:
Gets security subject ID initiated this task event if IgniteSecurity is
enabled, otherwise returns null.
The same is actual for CacheEvent, CacheQueryExecutedEvent and
CacheQueryReadEvent.
If there are no objections, I am going to create a relevant issue in Jira.
Re: JavaDoc for Event's subjectId methods
Posted by Denis Garus <ga...@gmail.com>.
Thank you, Maksim!
I think that the description should look like:
"Gets security subject ID initiated this task event if IgniteSecurity is
enabled, otherwise returns null.
This property is not available for GridEventType#EVT_TASK_SESSION_ATTR_SET
task event."
Please pay close attention that described behavior is confirmed by tests.
пн, 30 сент. 2019 г. в 13:11, Maksim Stepachev <ma...@gmail.com>:
> Denis,
>
> I added it in my ticket and pull-request. Should I change the only first
> sentence or full comment?
>
> пн, 30 сент. 2019 г. в 11:27, Denis Garus <ga...@gmail.com>:
>
>> Hello!
>>
>> I suggested to Maksim Stepachev include these changes in the scope of
>> your thicket [1]
>> and it looks like he agreed [2].
>>
>> Maksim Stepachev, could you please reflect JavaDoc and behavior changes
>> of events in your ticket?
>>
>> 1. https://issues.apache.org/jira/browse/IGNITE-11992
>> 2.
>> http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html
>>
>> пн, 30 сент. 2019 г. в 11:07, Ivan Pavlukhin <vo...@gmail.com>:
>>
>>> Hi,
>>>
>>> Do we allow commits to master without a ticket? I can imagine only
>>> reverts as an exception.
>>>
>>> Otherwise a ticket is a primary process item. Work description,
>>> review, CI checks (we have a job checking javadocs).
>>>
>>> ср, 25 сент. 2019 г. в 01:15, Denis Magda <dm...@apache.org>:
>>> >
>>> > Denis, please feel free to go and edit the JavaDocs in place without a
>>> > ticket. The changes suggested by you are reasonable.
>>> >
>>> > -
>>> > Denis
>>> >
>>> >
>>> > On Tue, Sep 24, 2019 at 3:55 AM Denis Garus <ga...@gmail.com>
>>> wrote:
>>> >
>>> > > Hello, Igniters!
>>> > >
>>> > > Some events contain the subjectId method, for example,
>>> TaskEvent#subjectId.
>>> > > The JavaDoc for this method is:
>>> > > "Gets security subject ID initiated this task event, if available.
>>> > > This property is not available for
>>> GridEventType#EVT_TASK_SESSION_ATTR_SET
>>> > > task event.
>>> > > Subject ID will be set either to node ID or client ID initiated task
>>> > > execution."
>>> > >
>>> > > I think It's wrong. The main point is a subject id doesn't have any
>>> sense
>>> > > if IgniteSecurity is disabled.
>>> > > However, if IgniteSecurity is enabled, the method must return the
>>> subject
>>> > > id from the current security context.
>>> > > Thus, the description (and behavior) of the method should be the
>>> following:
>>> > > Gets security subject ID initiated this task event if IgniteSecurity
>>> is
>>> > > enabled, otherwise returns null.
>>> > >
>>> > > The same is actual for CacheEvent, CacheQueryExecutedEvent and
>>> > > CacheQueryReadEvent.
>>> > >
>>> > > If there are no objections, I am going to create a relevant issue in
>>> Jira.
>>> > >
>>>
>>>
>>>
>>> --
>>> Best regards,
>>> Ivan Pavlukhin
>>>
>>
Re: JavaDoc for Event's subjectId methods
Posted by Maksim Stepachev <ma...@gmail.com>.
Denis,
I added it in my ticket and pull-request. Should I change the only first
sentence or full comment?
пн, 30 сент. 2019 г. в 11:27, Denis Garus <ga...@gmail.com>:
> Hello!
>
> I suggested to Maksim Stepachev include these changes in the scope of your
> thicket [1]
> and it looks like he agreed [2].
>
> Maksim Stepachev, could you please reflect JavaDoc and behavior changes of
> events in your ticket?
>
> 1. https://issues.apache.org/jira/browse/IGNITE-11992
> 2.
> http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html
>
> пн, 30 сент. 2019 г. в 11:07, Ivan Pavlukhin <vo...@gmail.com>:
>
>> Hi,
>>
>> Do we allow commits to master without a ticket? I can imagine only
>> reverts as an exception.
>>
>> Otherwise a ticket is a primary process item. Work description,
>> review, CI checks (we have a job checking javadocs).
>>
>> ср, 25 сент. 2019 г. в 01:15, Denis Magda <dm...@apache.org>:
>> >
>> > Denis, please feel free to go and edit the JavaDocs in place without a
>> > ticket. The changes suggested by you are reasonable.
>> >
>> > -
>> > Denis
>> >
>> >
>> > On Tue, Sep 24, 2019 at 3:55 AM Denis Garus <ga...@gmail.com>
>> wrote:
>> >
>> > > Hello, Igniters!
>> > >
>> > > Some events contain the subjectId method, for example,
>> TaskEvent#subjectId.
>> > > The JavaDoc for this method is:
>> > > "Gets security subject ID initiated this task event, if available.
>> > > This property is not available for
>> GridEventType#EVT_TASK_SESSION_ATTR_SET
>> > > task event.
>> > > Subject ID will be set either to node ID or client ID initiated task
>> > > execution."
>> > >
>> > > I think It's wrong. The main point is a subject id doesn't have any
>> sense
>> > > if IgniteSecurity is disabled.
>> > > However, if IgniteSecurity is enabled, the method must return the
>> subject
>> > > id from the current security context.
>> > > Thus, the description (and behavior) of the method should be the
>> following:
>> > > Gets security subject ID initiated this task event if IgniteSecurity
>> is
>> > > enabled, otherwise returns null.
>> > >
>> > > The same is actual for CacheEvent, CacheQueryExecutedEvent and
>> > > CacheQueryReadEvent.
>> > >
>> > > If there are no objections, I am going to create a relevant issue in
>> Jira.
>> > >
>>
>>
>>
>> --
>> Best regards,
>> Ivan Pavlukhin
>>
>
Re: JavaDoc for Event's subjectId methods
Posted by Denis Garus <ga...@gmail.com>.
Hello!
I suggested to Maksim Stepachev include these changes in the scope of your
thicket [1]
and it looks like he agreed [2].
Maksim Stepachev, could you please reflect JavaDoc and behavior changes of
events in your ticket?
1. https://issues.apache.org/jira/browse/IGNITE-11992
2.
http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html
пн, 30 сент. 2019 г. в 11:07, Ivan Pavlukhin <vo...@gmail.com>:
> Hi,
>
> Do we allow commits to master without a ticket? I can imagine only
> reverts as an exception.
>
> Otherwise a ticket is a primary process item. Work description,
> review, CI checks (we have a job checking javadocs).
>
> ср, 25 сент. 2019 г. в 01:15, Denis Magda <dm...@apache.org>:
> >
> > Denis, please feel free to go and edit the JavaDocs in place without a
> > ticket. The changes suggested by you are reasonable.
> >
> > -
> > Denis
> >
> >
> > On Tue, Sep 24, 2019 at 3:55 AM Denis Garus <ga...@gmail.com> wrote:
> >
> > > Hello, Igniters!
> > >
> > > Some events contain the subjectId method, for example,
> TaskEvent#subjectId.
> > > The JavaDoc for this method is:
> > > "Gets security subject ID initiated this task event, if available.
> > > This property is not available for
> GridEventType#EVT_TASK_SESSION_ATTR_SET
> > > task event.
> > > Subject ID will be set either to node ID or client ID initiated task
> > > execution."
> > >
> > > I think It's wrong. The main point is a subject id doesn't have any
> sense
> > > if IgniteSecurity is disabled.
> > > However, if IgniteSecurity is enabled, the method must return the
> subject
> > > id from the current security context.
> > > Thus, the description (and behavior) of the method should be the
> following:
> > > Gets security subject ID initiated this task event if IgniteSecurity is
> > > enabled, otherwise returns null.
> > >
> > > The same is actual for CacheEvent, CacheQueryExecutedEvent and
> > > CacheQueryReadEvent.
> > >
> > > If there are no objections, I am going to create a relevant issue in
> Jira.
> > >
>
>
>
> --
> Best regards,
> Ivan Pavlukhin
>
Re: JavaDoc for Event's subjectId methods
Posted by Ivan Pavlukhin <vo...@gmail.com>.
Hi,
Do we allow commits to master without a ticket? I can imagine only
reverts as an exception.
Otherwise a ticket is a primary process item. Work description,
review, CI checks (we have a job checking javadocs).
ср, 25 сент. 2019 г. в 01:15, Denis Magda <dm...@apache.org>:
>
> Denis, please feel free to go and edit the JavaDocs in place without a
> ticket. The changes suggested by you are reasonable.
>
> -
> Denis
>
>
> On Tue, Sep 24, 2019 at 3:55 AM Denis Garus <ga...@gmail.com> wrote:
>
> > Hello, Igniters!
> >
> > Some events contain the subjectId method, for example, TaskEvent#subjectId.
> > The JavaDoc for this method is:
> > "Gets security subject ID initiated this task event, if available.
> > This property is not available for GridEventType#EVT_TASK_SESSION_ATTR_SET
> > task event.
> > Subject ID will be set either to node ID or client ID initiated task
> > execution."
> >
> > I think It's wrong. The main point is a subject id doesn't have any sense
> > if IgniteSecurity is disabled.
> > However, if IgniteSecurity is enabled, the method must return the subject
> > id from the current security context.
> > Thus, the description (and behavior) of the method should be the following:
> > Gets security subject ID initiated this task event if IgniteSecurity is
> > enabled, otherwise returns null.
> >
> > The same is actual for CacheEvent, CacheQueryExecutedEvent and
> > CacheQueryReadEvent.
> >
> > If there are no objections, I am going to create a relevant issue in Jira.
> >
--
Best regards,
Ivan Pavlukhin
Re: JavaDoc for Event's subjectId methods
Posted by Denis Magda <dm...@apache.org>.
Denis, please feel free to go and edit the JavaDocs in place without a
ticket. The changes suggested by you are reasonable.
-
Denis
On Tue, Sep 24, 2019 at 3:55 AM Denis Garus <ga...@gmail.com> wrote:
> Hello, Igniters!
>
> Some events contain the subjectId method, for example, TaskEvent#subjectId.
> The JavaDoc for this method is:
> "Gets security subject ID initiated this task event, if available.
> This property is not available for GridEventType#EVT_TASK_SESSION_ATTR_SET
> task event.
> Subject ID will be set either to node ID or client ID initiated task
> execution."
>
> I think It's wrong. The main point is a subject id doesn't have any sense
> if IgniteSecurity is disabled.
> However, if IgniteSecurity is enabled, the method must return the subject
> id from the current security context.
> Thus, the description (and behavior) of the method should be the following:
> Gets security subject ID initiated this task event if IgniteSecurity is
> enabled, otherwise returns null.
>
> The same is actual for CacheEvent, CacheQueryExecutedEvent and
> CacheQueryReadEvent.
>
> If there are no objections, I am going to create a relevant issue in Jira.
>