You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2013/03/03 22:43:56 UTC

Review Request: Fixed ExecutorInfo comparision and example tests.

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

Review request for mesos, Benjamin Hindman and Ben Mahler.


Description
-------

See summary.


Diffs
-----

  src/common/type_utils.hpp fde69aeec403b3455839dca6b0b2e1507d81ba00 
  src/examples/balloon_framework.cpp 78f03b69147367844bbd2c3762574aa6f11e670b 
  src/examples/java/TestFramework.java dc892c7a6444d2940839529072258e151ea84d83 
  src/examples/python/test_framework.py bba1f508fdcff050c653f4b9de472a197ab8bb9f 
  src/examples/test_framework.cpp dd2dd575ec7287a922d7a7c9b4cc0e62c2e29e0e 

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


Testing
-------

make check.


Thanks,

Vinod Kone


Re: Review Request: Fixed ExecutorInfo comparision and example tests.

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

> On March 4, 2013, 6:20 p.m., Ben Mahler wrote:
> > src/common/type_utils.hpp, line 245
> > <https://reviews.apache.org/r/9723/diff/1/?file=264738#file264738line245>
> >
> >     can you compare the framework id as well, if present?

good catch. ty.


- Vinod


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


On March 3, 2013, 9:43 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9723/
> -----------------------------------------------------------
> 
> (Updated March 3, 2013, 9:43 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/common/type_utils.hpp fde69aeec403b3455839dca6b0b2e1507d81ba00 
>   src/examples/balloon_framework.cpp 78f03b69147367844bbd2c3762574aa6f11e670b 
>   src/examples/java/TestFramework.java dc892c7a6444d2940839529072258e151ea84d83 
>   src/examples/python/test_framework.py bba1f508fdcff050c653f4b9de472a197ab8bb9f 
>   src/examples/test_framework.cpp dd2dd575ec7287a922d7a7c9b4cc0e62c2e29e0e 
> 
> Diff: https://reviews.apache.org/r/9723/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request: Fixed ExecutorInfo comparision and example tests.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9723/#review17341
-----------------------------------------------------------


Thanks! I think we should have the source field express the semantics behind why we're launching the executor (as in my comments below).


src/common/type_utils.hpp
<https://reviews.apache.org/r/9723/#comment36849>

    can you compare the framework id as well, if present?



src/examples/balloon_framework.cpp
<https://reviews.apache.org/r/9723/#comment36843>

    I think this is misleading, let's call the source: "balloon_test"



src/examples/java/TestFramework.java
<https://reviews.apache.org/r/9723/#comment36846>

    Ditto, can you name the source "java_test" or the like?



src/examples/python/test_framework.py
<https://reviews.apache.org/r/9723/#comment36847>

    "python_test"?



src/examples/test_framework.cpp
<https://reviews.apache.org/r/9723/#comment36848>

    "cpp_test"?


- Ben Mahler


On March 3, 2013, 9:43 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9723/
> -----------------------------------------------------------
> 
> (Updated March 3, 2013, 9:43 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/common/type_utils.hpp fde69aeec403b3455839dca6b0b2e1507d81ba00 
>   src/examples/balloon_framework.cpp 78f03b69147367844bbd2c3762574aa6f11e670b 
>   src/examples/java/TestFramework.java dc892c7a6444d2940839529072258e151ea84d83 
>   src/examples/python/test_framework.py bba1f508fdcff050c653f4b9de472a197ab8bb9f 
>   src/examples/test_framework.cpp dd2dd575ec7287a922d7a7c9b4cc0e62c2e29e0e 
> 
> Diff: https://reviews.apache.org/r/9723/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request: Fixed ExecutorInfo comparision and example tests.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9723/
-----------------------------------------------------------

(Updated March 4, 2013, 10:24 p.m.)


Review request for mesos, Benjamin Hindman and Ben Mahler.


Changes
-------

corrected the bad rebase (diff3).


Description
-------

See summary.


Diffs (updated)
-----

  src/common/type_utils.hpp fde69aeec403b3455839dca6b0b2e1507d81ba00 
  src/examples/balloon_framework.cpp 78f03b69147367844bbd2c3762574aa6f11e670b 
  src/examples/java/TestFramework.java dc892c7a6444d2940839529072258e151ea84d83 
  src/examples/python/test_framework.py bba1f508fdcff050c653f4b9de472a197ab8bb9f 
  src/examples/test_framework.cpp dd2dd575ec7287a922d7a7c9b4cc0e62c2e29e0e 

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


Testing
-------

make check.


Thanks,

Vinod Kone


Re: Review Request: Fixed ExecutorInfo comparision and example tests.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9723/
-----------------------------------------------------------

(Updated March 4, 2013, 10:11 p.m.)


Review request for mesos, Benjamin Hindman and Ben Mahler.


Changes
-------

source's with all lower case and underscores.


Description
-------

See summary.


Diffs (updated)
-----

  src/common/type_utils.hpp fde69aeec403b3455839dca6b0b2e1507d81ba00 
  src/examples/balloon_framework.cpp 78f03b69147367844bbd2c3762574aa6f11e670b 
  src/examples/java/TestFramework.java dc892c7a6444d2940839529072258e151ea84d83 
  src/examples/python/test_framework.py bba1f508fdcff050c653f4b9de472a197ab8bb9f 
  src/examples/test_framework.cpp dd2dd575ec7287a922d7a7c9b4cc0e62c2e29e0e 
  src/files/files.cpp 75731a4379bf308fd812bb966b0653570ec3b349 
  src/master/drf_sorter.cpp 33a8ec8ba0c5d4189719feb0f4b001a91910bd35 
  src/master/hierarchical_allocator_process.hpp 33e059c10bb87db88b1fb9a100b7504bc89ac896 
  src/master/http.cpp 422ed853475f6f9c592fe28185250e3e71daad92 
  src/slave/cgroups_isolation_module.hpp a04fc46b15d2741886f5847cadbdc9bed351c588 
  src/slave/cgroups_isolation_module.cpp a779de80d13c67e507d7d2ee788fcdaa5e71daca 
  src/slave/slave.hpp 7648c33230c1900eda7529045c5df9ccab105d47 
  src/slave/slave.cpp c13d268c5e0e107902f64e30304a18128927a571 
  src/tests/allocator_tests.cpp ad3de212c6ce71d0f3dc14d6198a899a00a35bc8 
  src/tests/balloon_framework_test.sh 93a733f64cfde08349b7781eb3d5e13594c74498 
  src/tests/gc_tests.cpp 90b3a26ded877cff6eb6e29bf8139950ce237950 
  src/tests/master_tests.cpp 104610cef3fdaaea75271657c42c4fd2b6b61618 
  src/tests/utils.hpp b648b631d8c11c26e6adfa3f5b16012044557e25 

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


Testing
-------

make check.


Thanks,

Vinod Kone


Re: Review Request: Fixed ExecutorInfo comparision and example tests.

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

> On March 4, 2013, 7:03 p.m., Ben Mahler wrote:
> > src/examples/balloon_framework.cpp, line 201
> > <https://reviews.apache.org/r/9723/diff/2/?file=265168#file265168line201>
> >
> >     Given people might look at these examples, these files are advocating to framework designers on how to name the source field.
> >     
> >     So can we name them as "balloon_test"? That is, lower case, no strange characters, and underscores instead of dashes?

I don't get it. Is there any reason why they shouldn't have caps, dashes etc? Most modern programming languages accept all unicode characters as unicode. For eg: our master and slave id has dashes in it.


> On March 4, 2013, 7:03 p.m., Ben Mahler wrote:
> > src/examples/test_framework.cpp, line 186
> > <https://reviews.apache.org/r/9723/diff/2/?file=265171#file265171line186>
> >
> >     once we start validating identifier characters, we might be excluding '+' so for this one I think "cpp_test" is more idiomatic

why? we are not creating a filename in the system based on this system, are we?


- Vinod


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


On March 4, 2013, 6:57 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9723/
> -----------------------------------------------------------
> 
> (Updated March 4, 2013, 6:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/common/type_utils.hpp fde69aeec403b3455839dca6b0b2e1507d81ba00 
>   src/examples/balloon_framework.cpp 78f03b69147367844bbd2c3762574aa6f11e670b 
>   src/examples/java/TestFramework.java dc892c7a6444d2940839529072258e151ea84d83 
>   src/examples/python/test_framework.py bba1f508fdcff050c653f4b9de472a197ab8bb9f 
>   src/examples/test_framework.cpp dd2dd575ec7287a922d7a7c9b4cc0e62c2e29e0e 
> 
> Diff: https://reviews.apache.org/r/9723/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request: Fixed ExecutorInfo comparision and example tests.

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

> On March 4, 2013, 7:03 p.m., Ben Mahler wrote:
> > src/examples/balloon_framework.cpp, line 201
> > <https://reviews.apache.org/r/9723/diff/2/?file=265168#file265168line201>
> >
> >     Given people might look at these examples, these files are advocating to framework designers on how to name the source field.
> >     
> >     So can we name them as "balloon_test"? That is, lower case, no strange characters, and underscores instead of dashes?
> 
> Vinod Kone wrote:
>     I don't get it. Is there any reason why they shouldn't have caps, dashes etc? Most modern programming languages accept all unicode characters as unicode. For eg: our master and slave id has dashes in it.

s/as unicode/as identifiers/


- Vinod


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


On March 4, 2013, 6:57 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9723/
> -----------------------------------------------------------
> 
> (Updated March 4, 2013, 6:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/common/type_utils.hpp fde69aeec403b3455839dca6b0b2e1507d81ba00 
>   src/examples/balloon_framework.cpp 78f03b69147367844bbd2c3762574aa6f11e670b 
>   src/examples/java/TestFramework.java dc892c7a6444d2940839529072258e151ea84d83 
>   src/examples/python/test_framework.py bba1f508fdcff050c653f4b9de472a197ab8bb9f 
>   src/examples/test_framework.cpp dd2dd575ec7287a922d7a7c9b4cc0e62c2e29e0e 
> 
> Diff: https://reviews.apache.org/r/9723/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request: Fixed ExecutorInfo comparision and example tests.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9723/#review17346
-----------------------------------------------------------

Ship it!


Modulo the source id's. I think they should be idiomatic examples of identifiers, i.e. lowercase, underscores, no strange characters (+)


src/examples/balloon_framework.cpp
<https://reviews.apache.org/r/9723/#comment36852>

    Given people might look at these examples, these files are advocating to framework designers on how to name the source field.
    
    So can we name them as "balloon_test"? That is, lower case, no strange characters, and underscores instead of dashes?



src/examples/java/TestFramework.java
<https://reviews.apache.org/r/9723/#comment36854>

    Lowercase? java_test



src/examples/python/test_framework.py
<https://reviews.apache.org/r/9723/#comment36855>

    lowercase? python_test



src/examples/test_framework.cpp
<https://reviews.apache.org/r/9723/#comment36853>

    once we start validating identifier characters, we might be excluding '+' so for this one I think "cpp_test" is more idiomatic


- Ben Mahler


On March 4, 2013, 6:57 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9723/
> -----------------------------------------------------------
> 
> (Updated March 4, 2013, 6:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/common/type_utils.hpp fde69aeec403b3455839dca6b0b2e1507d81ba00 
>   src/examples/balloon_framework.cpp 78f03b69147367844bbd2c3762574aa6f11e670b 
>   src/examples/java/TestFramework.java dc892c7a6444d2940839529072258e151ea84d83 
>   src/examples/python/test_framework.py bba1f508fdcff050c653f4b9de472a197ab8bb9f 
>   src/examples/test_framework.cpp dd2dd575ec7287a922d7a7c9b4cc0e62c2e29e0e 
> 
> Diff: https://reviews.apache.org/r/9723/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request: Fixed ExecutorInfo comparision and example tests.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9723/
-----------------------------------------------------------

(Updated March 4, 2013, 6:57 p.m.)


Review request for mesos, Benjamin Hindman and Ben Mahler.


Changes
-------

benm's.


Description
-------

See summary.


Diffs (updated)
-----

  src/common/type_utils.hpp fde69aeec403b3455839dca6b0b2e1507d81ba00 
  src/examples/balloon_framework.cpp 78f03b69147367844bbd2c3762574aa6f11e670b 
  src/examples/java/TestFramework.java dc892c7a6444d2940839529072258e151ea84d83 
  src/examples/python/test_framework.py bba1f508fdcff050c653f4b9de472a197ab8bb9f 
  src/examples/test_framework.cpp dd2dd575ec7287a922d7a7c9b4cc0e62c2e29e0e 

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


Testing
-------

make check.


Thanks,

Vinod Kone