You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@mesos.apache.org by Jiang Yan Xu <ya...@jxu.me> on 2014/06/06 22:09:55 UTC

Review Request 22315: Updated Mesos codebase to set the framework principal in FrameworkInfo.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22315/
-----------------------------------------------------------

Review request for mesos, Benjamin Hindman and Vinod Kone.


Bugs: MESOS-1339
    https://issues.apache.org/jira/browse/MESOS-1339


Repository: mesos-git


Description
-------

See summary.


Diffs
-----

  src/examples/balloon_framework.cpp 400764dde2b648c05bd3621af6e992de69c5c7d3 
  src/examples/java/TestExceptionFramework.java 464b3b02e9f4a6d9b8d6f869b363c859ac826498 
  src/examples/java/TestFramework.java 65ee2dc8c4dba2234fe23f4c39c295167d4c5918 
  src/examples/java/TestMultipleExecutorsFramework.java 6846959fe1f12587c28f9380e41332dadf9b273d 
  src/examples/long_lived_framework.cpp 638e3162014283c77390e60fdf2a249cead92d63 
  src/examples/no_executor_framework.cpp 0cb987c23db64b781b9e47113b5e2936323cd5bb 
  src/examples/python/test_framework.py c37de6ed4b7f3d07ee461123393ed9df7b84155d 
  src/examples/test_framework.cpp 66ce3a645cedf155516458ab623b442374beaa3a 
  src/tests/allocator_tests.cpp 79ea09c8d05aa3ac3c035ea3964858bff601e213 

Diff: https://reviews.apache.org/r/22315/diff/


Testing
-------

make check.


Thanks,

Jiang Yan Xu

Re: Review Request 22315: Updated Mesos codebase to set the framework principal in FrameworkInfo.

Posted by Vinod Kone <vi...@gmail.com>.


> On June 6, 2014, 11:31 p.m., Vinod Kone wrote:
> > src/tests/allocator_tests.cpp, line 99
> > <https://reviews.apache.org/r/22315/diff/1/?file=604887#file604887line99>
> >
> >     so you are doing this just to get the default principal?
> >     
> >     perhaps you can just set it explicitly? you could actually make DEFAULT_PRINCIPAL a macro and have DEFAULT_CREDENTIAL use that.
> 
> Jiang Yan Xu wrote:
>     These are what I found the only places FrameworkInfos are not constructed from DEFAULT_FRAMEWORK_INFO. I think this is more future proof in that if we add something to DEFAULT_FRAMEWORK_INFO in the future these don't need to be updated again unless we want to change the tests.
> 
> Vinod Kone wrote:
>
> 
> Jiang Yan Xu wrote:
>     Did you intend to comment here?

no. if this is how we create framework infos everywhere else, then its fine by me.


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22315/#review44979
-----------------------------------------------------------


On June 6, 2014, 8:09 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22315/
> -----------------------------------------------------------
> 
> (Updated June 6, 2014, 8:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Bugs: MESOS-1339
>     https://issues.apache.org/jira/browse/MESOS-1339
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/examples/balloon_framework.cpp 400764dde2b648c05bd3621af6e992de69c5c7d3 
>   src/examples/java/TestExceptionFramework.java 464b3b02e9f4a6d9b8d6f869b363c859ac826498 
>   src/examples/java/TestFramework.java 65ee2dc8c4dba2234fe23f4c39c295167d4c5918 
>   src/examples/java/TestMultipleExecutorsFramework.java 6846959fe1f12587c28f9380e41332dadf9b273d 
>   src/examples/long_lived_framework.cpp 638e3162014283c77390e60fdf2a249cead92d63 
>   src/examples/no_executor_framework.cpp 0cb987c23db64b781b9e47113b5e2936323cd5bb 
>   src/examples/python/test_framework.py c37de6ed4b7f3d07ee461123393ed9df7b84155d 
>   src/examples/test_framework.cpp 66ce3a645cedf155516458ab623b442374beaa3a 
>   src/tests/allocator_tests.cpp 79ea09c8d05aa3ac3c035ea3964858bff601e213 
> 
> Diff: https://reviews.apache.org/r/22315/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 22315: Updated Mesos codebase to set the framework principal in FrameworkInfo.

Posted by Vinod Kone <vi...@gmail.com>.


> On June 6, 2014, 11:31 p.m., Vinod Kone wrote:
> > src/examples/balloon_framework.cpp, lines 247-252
> > <https://reviews.apache.org/r/22315/diff/1/?file=604879#file604879line247>
> >
> >     use the same principal for both auth and no-auth case to avoid confusion. here and everywhere else.
> 
> Jiang Yan Xu wrote:
>     But from the context it's not perfectly clear what the "DEFAULT_PRINCIPAL" is. e.g. If they are not run by make check you need to set them. They are defined as "examples" and not "tests" :) To understand where "test-principal" comes from I need to locate it in "tests/mesos.hpp".

Oh. It's a getenv(). Never mind. You can drop this.


> On June 6, 2014, 11:31 p.m., Vinod Kone wrote:
> > src/tests/allocator_tests.cpp, line 99
> > <https://reviews.apache.org/r/22315/diff/1/?file=604887#file604887line99>
> >
> >     so you are doing this just to get the default principal?
> >     
> >     perhaps you can just set it explicitly? you could actually make DEFAULT_PRINCIPAL a macro and have DEFAULT_CREDENTIAL use that.
> 
> Jiang Yan Xu wrote:
>     These are what I found the only places FrameworkInfos are not constructed from DEFAULT_FRAMEWORK_INFO. I think this is more future proof in that if we add something to DEFAULT_FRAMEWORK_INFO in the future these don't need to be updated again unless we want to change the tests.


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22315/#review44979
-----------------------------------------------------------


On June 6, 2014, 8:09 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22315/
> -----------------------------------------------------------
> 
> (Updated June 6, 2014, 8:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Bugs: MESOS-1339
>     https://issues.apache.org/jira/browse/MESOS-1339
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/examples/balloon_framework.cpp 400764dde2b648c05bd3621af6e992de69c5c7d3 
>   src/examples/java/TestExceptionFramework.java 464b3b02e9f4a6d9b8d6f869b363c859ac826498 
>   src/examples/java/TestFramework.java 65ee2dc8c4dba2234fe23f4c39c295167d4c5918 
>   src/examples/java/TestMultipleExecutorsFramework.java 6846959fe1f12587c28f9380e41332dadf9b273d 
>   src/examples/long_lived_framework.cpp 638e3162014283c77390e60fdf2a249cead92d63 
>   src/examples/no_executor_framework.cpp 0cb987c23db64b781b9e47113b5e2936323cd5bb 
>   src/examples/python/test_framework.py c37de6ed4b7f3d07ee461123393ed9df7b84155d 
>   src/examples/test_framework.cpp 66ce3a645cedf155516458ab623b442374beaa3a 
>   src/tests/allocator_tests.cpp 79ea09c8d05aa3ac3c035ea3964858bff601e213 
> 
> Diff: https://reviews.apache.org/r/22315/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 22315: Updated Mesos codebase to set the framework principal in FrameworkInfo.

Posted by Jiang Yan Xu <ya...@jxu.me>.


> On June 6, 2014, 4:31 p.m., Vinod Kone wrote:
> > src/tests/allocator_tests.cpp, line 99
> > <https://reviews.apache.org/r/22315/diff/1/?file=604887#file604887line99>
> >
> >     so you are doing this just to get the default principal?
> >     
> >     perhaps you can just set it explicitly? you could actually make DEFAULT_PRINCIPAL a macro and have DEFAULT_CREDENTIAL use that.
> 
> Jiang Yan Xu wrote:
>     These are what I found the only places FrameworkInfos are not constructed from DEFAULT_FRAMEWORK_INFO. I think this is more future proof in that if we add something to DEFAULT_FRAMEWORK_INFO in the future these don't need to be updated again unless we want to change the tests.
> 
> Vinod Kone wrote:
>

Did you intend to comment here?


- Jiang Yan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22315/#review44979
-----------------------------------------------------------


On June 6, 2014, 1:09 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22315/
> -----------------------------------------------------------
> 
> (Updated June 6, 2014, 1:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Bugs: MESOS-1339
>     https://issues.apache.org/jira/browse/MESOS-1339
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/examples/balloon_framework.cpp 400764dde2b648c05bd3621af6e992de69c5c7d3 
>   src/examples/java/TestExceptionFramework.java 464b3b02e9f4a6d9b8d6f869b363c859ac826498 
>   src/examples/java/TestFramework.java 65ee2dc8c4dba2234fe23f4c39c295167d4c5918 
>   src/examples/java/TestMultipleExecutorsFramework.java 6846959fe1f12587c28f9380e41332dadf9b273d 
>   src/examples/long_lived_framework.cpp 638e3162014283c77390e60fdf2a249cead92d63 
>   src/examples/no_executor_framework.cpp 0cb987c23db64b781b9e47113b5e2936323cd5bb 
>   src/examples/python/test_framework.py c37de6ed4b7f3d07ee461123393ed9df7b84155d 
>   src/examples/test_framework.cpp 66ce3a645cedf155516458ab623b442374beaa3a 
>   src/tests/allocator_tests.cpp 79ea09c8d05aa3ac3c035ea3964858bff601e213 
> 
> Diff: https://reviews.apache.org/r/22315/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 22315: Updated Mesos codebase to set the framework principal in FrameworkInfo.

Posted by Jiang Yan Xu <ya...@jxu.me>.


> On June 6, 2014, 4:31 p.m., Vinod Kone wrote:
> > src/tests/allocator_tests.cpp, line 99
> > <https://reviews.apache.org/r/22315/diff/1/?file=604887#file604887line99>
> >
> >     so you are doing this just to get the default principal?
> >     
> >     perhaps you can just set it explicitly? you could actually make DEFAULT_PRINCIPAL a macro and have DEFAULT_CREDENTIAL use that.

These are what I found the only places FrameworkInfos are not constructed from DEFAULT_FRAMEWORK_INFO. I think this is more future proof in that if we add something to DEFAULT_FRAMEWORK_INFO in the future these don't need to be updated again unless we want to change the tests.


> On June 6, 2014, 4:31 p.m., Vinod Kone wrote:
> > src/examples/balloon_framework.cpp, lines 247-252
> > <https://reviews.apache.org/r/22315/diff/1/?file=604879#file604879line247>
> >
> >     use the same principal for both auth and no-auth case to avoid confusion. here and everywhere else.

But from the context it's not perfectly clear what the "DEFAULT_PRINCIPAL" is. e.g. If they are not run by make check you need to set them. They are defined as "examples" and not "tests" :) To understand where "test-principal" comes from I need to locate it in "tests/mesos.hpp".


- Jiang Yan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22315/#review44979
-----------------------------------------------------------


On June 6, 2014, 1:09 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22315/
> -----------------------------------------------------------
> 
> (Updated June 6, 2014, 1:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Bugs: MESOS-1339
>     https://issues.apache.org/jira/browse/MESOS-1339
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/examples/balloon_framework.cpp 400764dde2b648c05bd3621af6e992de69c5c7d3 
>   src/examples/java/TestExceptionFramework.java 464b3b02e9f4a6d9b8d6f869b363c859ac826498 
>   src/examples/java/TestFramework.java 65ee2dc8c4dba2234fe23f4c39c295167d4c5918 
>   src/examples/java/TestMultipleExecutorsFramework.java 6846959fe1f12587c28f9380e41332dadf9b273d 
>   src/examples/long_lived_framework.cpp 638e3162014283c77390e60fdf2a249cead92d63 
>   src/examples/no_executor_framework.cpp 0cb987c23db64b781b9e47113b5e2936323cd5bb 
>   src/examples/python/test_framework.py c37de6ed4b7f3d07ee461123393ed9df7b84155d 
>   src/examples/test_framework.cpp 66ce3a645cedf155516458ab623b442374beaa3a 
>   src/tests/allocator_tests.cpp 79ea09c8d05aa3ac3c035ea3964858bff601e213 
> 
> Diff: https://reviews.apache.org/r/22315/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 22315: Updated Mesos codebase to set the framework principal in FrameworkInfo.

Posted by Vinod Kone <vi...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22315/#review44979
-----------------------------------------------------------



src/examples/balloon_framework.cpp
<https://reviews.apache.org/r/22315/#comment79591>

    use the same principal for both auth and no-auth case to avoid confusion. here and everywhere else.



src/examples/no_executor_framework.cpp
<https://reviews.apache.org/r/22315/#comment79590>

    ditto.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/22315/#comment79593>

    so you are doing this just to get the default principal?
    
    perhaps you can just set it explicitly? you could actually make DEFAULT_PRINCIPAL a macro and have DEFAULT_CREDENTIAL use that.


- Vinod Kone


On June 6, 2014, 8:09 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22315/
> -----------------------------------------------------------
> 
> (Updated June 6, 2014, 8:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Bugs: MESOS-1339
>     https://issues.apache.org/jira/browse/MESOS-1339
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/examples/balloon_framework.cpp 400764dde2b648c05bd3621af6e992de69c5c7d3 
>   src/examples/java/TestExceptionFramework.java 464b3b02e9f4a6d9b8d6f869b363c859ac826498 
>   src/examples/java/TestFramework.java 65ee2dc8c4dba2234fe23f4c39c295167d4c5918 
>   src/examples/java/TestMultipleExecutorsFramework.java 6846959fe1f12587c28f9380e41332dadf9b273d 
>   src/examples/long_lived_framework.cpp 638e3162014283c77390e60fdf2a249cead92d63 
>   src/examples/no_executor_framework.cpp 0cb987c23db64b781b9e47113b5e2936323cd5bb 
>   src/examples/python/test_framework.py c37de6ed4b7f3d07ee461123393ed9df7b84155d 
>   src/examples/test_framework.cpp 66ce3a645cedf155516458ab623b442374beaa3a 
>   src/tests/allocator_tests.cpp 79ea09c8d05aa3ac3c035ea3964858bff601e213 
> 
> Diff: https://reviews.apache.org/r/22315/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 22315: Updated Mesos codebase to set the framework principal in FrameworkInfo.

Posted by Mesos ReviewBot <de...@mesos.apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22315/#review45033
-----------------------------------------------------------


Bad patch!

Reviews applied: [22315]

Failed command: git apply --index 22315.patch

Error:
 error: patch failed: src/tests/allocator_tests.cpp:972
error: src/tests/allocator_tests.cpp: patch does not apply


- Mesos ReviewBot


On June 6, 2014, 8:09 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22315/
> -----------------------------------------------------------
> 
> (Updated June 6, 2014, 8:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Bugs: MESOS-1339
>     https://issues.apache.org/jira/browse/MESOS-1339
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/examples/balloon_framework.cpp 400764dde2b648c05bd3621af6e992de69c5c7d3 
>   src/examples/java/TestExceptionFramework.java 464b3b02e9f4a6d9b8d6f869b363c859ac826498 
>   src/examples/java/TestFramework.java 65ee2dc8c4dba2234fe23f4c39c295167d4c5918 
>   src/examples/java/TestMultipleExecutorsFramework.java 6846959fe1f12587c28f9380e41332dadf9b273d 
>   src/examples/long_lived_framework.cpp 638e3162014283c77390e60fdf2a249cead92d63 
>   src/examples/no_executor_framework.cpp 0cb987c23db64b781b9e47113b5e2936323cd5bb 
>   src/examples/python/test_framework.py c37de6ed4b7f3d07ee461123393ed9df7b84155d 
>   src/examples/test_framework.cpp 66ce3a645cedf155516458ab623b442374beaa3a 
>   src/tests/allocator_tests.cpp 79ea09c8d05aa3ac3c035ea3964858bff601e213 
> 
> Diff: https://reviews.apache.org/r/22315/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 22315: Updated Mesos codebase to set the framework principal in FrameworkInfo.

Posted by Vinod Kone <vi...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22315/#review45088
-----------------------------------------------------------

Ship it!


Ship It!

- Vinod Kone


On June 6, 2014, 8:09 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22315/
> -----------------------------------------------------------
> 
> (Updated June 6, 2014, 8:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Bugs: MESOS-1339
>     https://issues.apache.org/jira/browse/MESOS-1339
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/examples/balloon_framework.cpp 400764dde2b648c05bd3621af6e992de69c5c7d3 
>   src/examples/java/TestExceptionFramework.java 464b3b02e9f4a6d9b8d6f869b363c859ac826498 
>   src/examples/java/TestFramework.java 65ee2dc8c4dba2234fe23f4c39c295167d4c5918 
>   src/examples/java/TestMultipleExecutorsFramework.java 6846959fe1f12587c28f9380e41332dadf9b273d 
>   src/examples/long_lived_framework.cpp 638e3162014283c77390e60fdf2a249cead92d63 
>   src/examples/no_executor_framework.cpp 0cb987c23db64b781b9e47113b5e2936323cd5bb 
>   src/examples/python/test_framework.py c37de6ed4b7f3d07ee461123393ed9df7b84155d 
>   src/examples/test_framework.cpp 66ce3a645cedf155516458ab623b442374beaa3a 
>   src/tests/allocator_tests.cpp 79ea09c8d05aa3ac3c035ea3964858bff601e213 
> 
> Diff: https://reviews.apache.org/r/22315/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>