You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2012/08/01 18:01:41 UTC

Re: Review Request: Added a Java in-memory implementation of State.

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

(Updated Aug. 1, 2012, 4:01 p.m.)


Review request for mesos, John Sirois and Florian Leibert.


Changes
-------

Refactored Java implementation of State to use immutable Variable's and cleaned up InMemoryState implementation.


Description
-------

See summary.


Diffs (updated)
-----

  src/Makefile.am 10f1101 
  src/java/jni/org_apache_mesos_state_Variable.cpp PRE-CREATION 
  src/java/jni/org_apache_mesos_state_ZooKeeperState.cpp PRE-CREATION 
  src/java/src/org/apache/mesos/state/InMemoryState.java PRE-CREATION 
  src/java/src/org/apache/mesos/state/State.java PRE-CREATION 
  src/java/src/org/apache/mesos/state/Variable.java PRE-CREATION 
  src/java/src/org/apache/mesos/state/ZooKeeperState.java PRE-CREATION 
  src/state/state.hpp e1d9c01 

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


Testing
-------

make check


Thanks,

Benjamin Hindman


Re: Review Request: Added a Java in-memory implementation of State.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On Aug. 2, 2012, 8:40 p.m., John Sirois wrote:
> >

Thanks for these reviews guys, submitting!


- Benjamin


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


On Aug. 1, 2012, 4:01 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6079/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2012, 4:01 p.m.)
> 
> 
> Review request for mesos, John Sirois and Florian Leibert.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 10f1101 
>   src/java/jni/org_apache_mesos_state_Variable.cpp PRE-CREATION 
>   src/java/jni/org_apache_mesos_state_ZooKeeperState.cpp PRE-CREATION 
>   src/java/src/org/apache/mesos/state/InMemoryState.java PRE-CREATION 
>   src/java/src/org/apache/mesos/state/State.java PRE-CREATION 
>   src/java/src/org/apache/mesos/state/Variable.java PRE-CREATION 
>   src/java/src/org/apache/mesos/state/ZooKeeperState.java PRE-CREATION 
>   src/state/state.hpp e1d9c01 
> 
> Diff: https://reviews.apache.org/r/6079/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request: Added a Java in-memory implementation of State.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On Aug. 2, 2012, 8:40 p.m., John Sirois wrote:
> > src/java/src/org/apache/mesos/state/InMemoryState.java, line 48
> > <https://reviews.apache.org/r/6079/diff/7/?file=132302#file132302line48>
> >
> >     this cast is not needed

A cast or InMemoryState.<Variable>futureFrom is required, I decided the cast was more readable.


> On Aug. 2, 2012, 8:40 p.m., John Sirois wrote:
> > src/java/src/org/apache/mesos/state/InMemoryState.java, line 69
> > <https://reviews.apache.org/r/6079/diff/7/?file=132302#file132302line69>
> >
> >     this(entry, null);
> >     
> >     or even:
> >     this(entry, entry.value);
> >     
> >     And simplify the value() implementation below.

Done.


> On Aug. 2, 2012, 8:40 p.m., John Sirois wrote:
> > src/java/src/org/apache/mesos/state/InMemoryState.java, line 125
> > <https://reviews.apache.org/r/6079/diff/7/?file=132302#file132302line125>
> >
> >     final

Done.


- Benjamin


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


On Aug. 1, 2012, 4:01 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6079/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2012, 4:01 p.m.)
> 
> 
> Review request for mesos, John Sirois and Florian Leibert.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 10f1101 
>   src/java/jni/org_apache_mesos_state_Variable.cpp PRE-CREATION 
>   src/java/jni/org_apache_mesos_state_ZooKeeperState.cpp PRE-CREATION 
>   src/java/src/org/apache/mesos/state/InMemoryState.java PRE-CREATION 
>   src/java/src/org/apache/mesos/state/State.java PRE-CREATION 
>   src/java/src/org/apache/mesos/state/Variable.java PRE-CREATION 
>   src/java/src/org/apache/mesos/state/ZooKeeperState.java PRE-CREATION 
>   src/state/state.hpp e1d9c01 
> 
> Diff: https://reviews.apache.org/r/6079/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request: Added a Java in-memory implementation of State.

Posted by John Sirois <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6079/#review9782
-----------------------------------------------------------

Ship it!



src/java/src/org/apache/mesos/state/InMemoryState.java
<https://reviews.apache.org/r/6079/#comment20768>

    this cast is not needed



src/java/src/org/apache/mesos/state/InMemoryState.java
<https://reviews.apache.org/r/6079/#comment20769>

    ditto



src/java/src/org/apache/mesos/state/InMemoryState.java
<https://reviews.apache.org/r/6079/#comment20772>

    this(entry, null);
    
    or even:
    this(entry, entry.value);
    
    And simplify the value() implementation below.



src/java/src/org/apache/mesos/state/InMemoryState.java
<https://reviews.apache.org/r/6079/#comment20775>

    final


- John Sirois


On Aug. 1, 2012, 4:01 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6079/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2012, 4:01 p.m.)
> 
> 
> Review request for mesos, John Sirois and Florian Leibert.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 10f1101 
>   src/java/jni/org_apache_mesos_state_Variable.cpp PRE-CREATION 
>   src/java/jni/org_apache_mesos_state_ZooKeeperState.cpp PRE-CREATION 
>   src/java/src/org/apache/mesos/state/InMemoryState.java PRE-CREATION 
>   src/java/src/org/apache/mesos/state/State.java PRE-CREATION 
>   src/java/src/org/apache/mesos/state/Variable.java PRE-CREATION 
>   src/java/src/org/apache/mesos/state/ZooKeeperState.java PRE-CREATION 
>   src/state/state.hpp e1d9c01 
> 
> Diff: https://reviews.apache.org/r/6079/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request: Added a Java in-memory implementation of State.

Posted by Florian Leibert <fl...@leibert.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6079/#review9693
-----------------------------------------------------------

Ship it!


Ship It!

- Florian Leibert


On Aug. 1, 2012, 4:01 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6079/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2012, 4:01 p.m.)
> 
> 
> Review request for mesos, John Sirois and Florian Leibert.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 10f1101 
>   src/java/jni/org_apache_mesos_state_Variable.cpp PRE-CREATION 
>   src/java/jni/org_apache_mesos_state_ZooKeeperState.cpp PRE-CREATION 
>   src/java/src/org/apache/mesos/state/InMemoryState.java PRE-CREATION 
>   src/java/src/org/apache/mesos/state/State.java PRE-CREATION 
>   src/java/src/org/apache/mesos/state/Variable.java PRE-CREATION 
>   src/java/src/org/apache/mesos/state/ZooKeeperState.java PRE-CREATION 
>   src/state/state.hpp e1d9c01 
> 
> Diff: https://reviews.apache.org/r/6079/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>