You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Вадим Опольский <va...@gmail.com> on 2017/04/17 12:23:18 UTC
IGNITE-4052 ready for review
Hi, Nikolay!
I've made the following improvements (
https://github.com/apache/ignite/pull/1783 ):
1) Moved framework builder related code to separate method, so it make code
cleaner and framework testable.
2) Call framework builder method from test to test role and user.
3) Added validating mesos role according with mesos role documentation
http://mesos.apache.org/documentation/latest/roles/
4) Still using setEnv method because in IgniteFramework we cannot override
static method and make mock static methods (except powermock, but it would
require 3 extra dependencies in the module).
P.S. setEnv method designed to worked both on Linux and Windows, and it
does not left variable in system environment after testing.
Vadim Opolski
2017-04-14 12:07 GMT+03:00 Nikolay Tikhonov (JIRA) <ji...@apache.org>:
>
> [ https://issues.apache.org/jira/browse/IGNITE-4052?page=
> com.atlassian.jira.plugin.system.issuetabpanels:comment-
> tabpanel&focusedCommentId=15968814#comment-15968814 ]
>
> Nikolay Tikhonov commented on IGNITE-4052:
> ------------------------------------------
>
> [~javaller]
> It meens that lets remove #setEnv mathod and will create mock in test
> which will override {{getUser}} and {{getRole}} methods. Also how do you
> think might be need to add validation for role? Which valid set of values
> for this property?
>
> > Add ability to set up users for MESOS
> > -------------------------------------
> >
> > Key: IGNITE-4052
> > URL: https://issues.apache.org/jira/browse/IGNITE-4052
> > Project: Ignite
> > Issue Type: Improvement
> > Components: general
> > Affects Versions: 1.7
> > Reporter: Nikolay Tikhonov
> > Assignee: Vadim Opolski
> > Priority: Trivial
> >
> > In current implementation Ignite Mesos Framework connects to MESOS
> cluster via current user. Need to add ability to configure this parameters
> via system env properties. Also need to add properties for mesos role.
> > See org/apache/ignite/mesos/IgniteFramework.java:537
>
>
>
> --
> This message was sent by Atlassian JIRA
> (v6.3.15#6346)
>
Re: IGNITE-4052 ready for review
Posted by Вадим Опольский <va...@gmail.com>.
Hi Nikolay!
The comments was added.
Vadim
2017-04-17 15:32 GMT+03:00 Nikolai Tikhonov <nt...@apache.org>:
> Vadim,
>
> Thank you for your contribution. I'll look at changes. Can you please post
> the list improvements to jira ticket?
>
> On Mon, Apr 17, 2017 at 3:23 PM, Вадим Опольский <va...@gmail.com>
> wrote:
>
>> Hi, Nikolay!
>>
>> I've made the following improvements ( https://github.com/apache/igni
>> te/pull/1783 ):
>>
>> 1) Moved framework builder related code to separate method, so it make
>> code cleaner and framework testable.
>>
>> 2) Call framework builder method from test to test role and user.
>>
>> 3) Added validating mesos role according with mesos role documentation
>> http://mesos.apache.org/documentation/latest/roles/
>>
>> 4) Still using setEnv method because in IgniteFramework we cannot
>> override static method and make mock static methods (except powermock, but
>> it would require 3 extra dependencies in the module).
>>
>> P.S. setEnv method designed to worked both on Linux and Windows, and it
>> does not left variable in system environment after testing.
>>
>> Vadim Opolski
>>
>>
>> 2017-04-14 12:07 GMT+03:00 Nikolay Tikhonov (JIRA) <ji...@apache.org>:
>>
>>>
>>> [ https://issues.apache.org/jira/browse/IGNITE-4052?page=com.a
>>> tlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&
>>> focusedCommentId=15968814#comment-15968814 ]
>>>
>>> Nikolay Tikhonov commented on IGNITE-4052:
>>> ------------------------------------------
>>>
>>> [~javaller]
>>> It meens that lets remove #setEnv mathod and will create mock in test
>>> which will override {{getUser}} and {{getRole}} methods. Also how do you
>>> think might be need to add validation for role? Which valid set of values
>>> for this property?
>>>
>>> > Add ability to set up users for MESOS
>>> > -------------------------------------
>>> >
>>> > Key: IGNITE-4052
>>> > URL: https://issues.apache.org/jira/browse/IGNITE-4052
>>> > Project: Ignite
>>> > Issue Type: Improvement
>>> > Components: general
>>> > Affects Versions: 1.7
>>> > Reporter: Nikolay Tikhonov
>>> > Assignee: Vadim Opolski
>>> > Priority: Trivial
>>> >
>>> > In current implementation Ignite Mesos Framework connects to MESOS
>>> cluster via current user. Need to add ability to configure this parameters
>>> via system env properties. Also need to add properties for mesos role.
>>> > See org/apache/ignite/mesos/IgniteFramework.java:537
>>>
>>>
>>>
>>> --
>>> This message was sent by Atlassian JIRA
>>> (v6.3.15#6346)
>>>
>>
>>
>
Re: IGNITE-4052 ready for review
Posted by Nikolai Tikhonov <nt...@apache.org>.
Vadim,
Thank you for your contribution. I'll look at changes. Can you please post
the list improvements to jira ticket?
On Mon, Apr 17, 2017 at 3:23 PM, Вадим Опольский <va...@gmail.com>
wrote:
> Hi, Nikolay!
>
> I've made the following improvements ( https://github.com/apache/
> ignite/pull/1783 ):
>
> 1) Moved framework builder related code to separate method, so it make
> code cleaner and framework testable.
>
> 2) Call framework builder method from test to test role and user.
>
> 3) Added validating mesos role according with mesos role documentation
> http://mesos.apache.org/documentation/latest/roles/
>
> 4) Still using setEnv method because in IgniteFramework we cannot override
> static method and make mock static methods (except powermock, but it would
> require 3 extra dependencies in the module).
>
> P.S. setEnv method designed to worked both on Linux and Windows, and it
> does not left variable in system environment after testing.
>
> Vadim Opolski
>
>
> 2017-04-14 12:07 GMT+03:00 Nikolay Tikhonov (JIRA) <ji...@apache.org>:
>
>>
>> [ https://issues.apache.org/jira/browse/IGNITE-4052?page=com.
>> atlassian.jira.plugin.system.issuetabpanels:comment-tabpane
>> l&focusedCommentId=15968814#comment-15968814 ]
>>
>> Nikolay Tikhonov commented on IGNITE-4052:
>> ------------------------------------------
>>
>> [~javaller]
>> It meens that lets remove #setEnv mathod and will create mock in test
>> which will override {{getUser}} and {{getRole}} methods. Also how do you
>> think might be need to add validation for role? Which valid set of values
>> for this property?
>>
>> > Add ability to set up users for MESOS
>> > -------------------------------------
>> >
>> > Key: IGNITE-4052
>> > URL: https://issues.apache.org/jira/browse/IGNITE-4052
>> > Project: Ignite
>> > Issue Type: Improvement
>> > Components: general
>> > Affects Versions: 1.7
>> > Reporter: Nikolay Tikhonov
>> > Assignee: Vadim Opolski
>> > Priority: Trivial
>> >
>> > In current implementation Ignite Mesos Framework connects to MESOS
>> cluster via current user. Need to add ability to configure this parameters
>> via system env properties. Also need to add properties for mesos role.
>> > See org/apache/ignite/mesos/IgniteFramework.java:537
>>
>>
>>
>> --
>> This message was sent by Atlassian JIRA
>> (v6.3.15#6346)
>>
>
>