Re: Review Request 47521: Added creator principal to '/create-volumes' tests.

2016-05-27 Thread Bernd Mathiske 

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


Ship it!




Ship It!

- Bernd Mathiske


On May 27, 2016, 3:55 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47521/
> ---
> 
> (Updated May 27, 2016, 3:55 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A creator principal is added to the persistent volumes
> used in the PersistentVolumeEndpointsTests.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> a57461d881b2bf0175f83b50b0a46167acd5bd3e 
> 
> Diff: https://reviews.apache.org/r/47521/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47521: Added creator principal to '/create-volumes' tests.

2016-05-27 Thread Greg Mann 

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

(Updated May 27, 2016, 10:55 a.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.


Changes
---

Removed unnecessary reserver principals.


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


Repository: mesos


Description
---

A creator principal is added to the persistent volumes
used in the PersistentVolumeEndpointsTests.


Diffs (updated)
-

  src/tests/persistent_volume_endpoints_tests.cpp 
a57461d881b2bf0175f83b50b0a46167acd5bd3e 

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


Testing
---

`make check` was used to test on OSX.


Thanks,

Greg Mann

Re: Review Request 47521: Added creator principal to '/create-volumes' tests.

2016-05-27 Thread Greg Mann 


> On May 27, 2016, 9:36 a.m., Bernd Mathiske wrote:
> > src/tests/persistent_volume_endpoints_tests.cpp, line 1011
> > 
> >
> > In the above case, this part is within the scope block, here it is not. 
> > Please move the opening curly brace up.

The volume is left out of the scope here because it gets used much later in the 
test. If we scoped it, we would have to extend the scope nearly to the end of 
the test. What do you think?


- Greg


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


On May 27, 2016, 5:37 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47521/
> ---
> 
> (Updated May 27, 2016, 5:37 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A creator principal is added to the persistent volumes
> used in the PersistentVolumeEndpointsTests.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> a57461d881b2bf0175f83b50b0a46167acd5bd3e 
> 
> Diff: https://reviews.apache.org/r/47521/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47521: Added creator principal to '/create-volumes' tests.

2016-05-27 Thread Bernd Mathiske 

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




src/tests/persistent_volume_endpoints_tests.cpp (line 660)


I don't think the reservation principal should be necessary here. Should 
not have any effect either way, right? But the documentation for this 
recommends not setting a reservation principal unless there is a reservation. 
Also I think we can avoid confusing by using None() here instead, so it is 
clear which parameter triggers the expected response.



src/tests/persistent_volume_endpoints_tests.cpp (line 718)


Same here.



src/tests/persistent_volume_endpoints_tests.cpp (line 1010)


In the above case, this part is within the scope block, here it is not. 
Please move the opening curly brace up.


- Bernd Mathiske


On May 26, 2016, 10:37 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47521/
> ---
> 
> (Updated May 26, 2016, 10:37 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A creator principal is added to the persistent volumes
> used in the PersistentVolumeEndpointsTests.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> a57461d881b2bf0175f83b50b0a46167acd5bd3e 
> 
> Diff: https://reviews.apache.org/r/47521/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47521: Added creator principal to '/create-volumes' tests.

2016-05-26 Thread Greg Mann 


> On May 26, 2016, 9:40 p.m., Greg Mann wrote:
> > src/tests/persistent_volume_endpoints_tests.cpp, line 1597
> > 
> >
> > perhaps use a variable for the principal in a subsequent patch?

Added a TODO for this.


- Greg


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


On May 27, 2016, 5:37 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47521/
> ---
> 
> (Updated May 27, 2016, 5:37 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A creator principal is added to the persistent volumes
> used in the PersistentVolumeEndpointsTests.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> a57461d881b2bf0175f83b50b0a46167acd5bd3e 
> 
> Diff: https://reviews.apache.org/r/47521/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47521: Added creator principal to '/create-volumes' tests.

2016-05-26 Thread Greg Mann 

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

(Updated May 27, 2016, 5:37 a.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

A creator principal is added to the persistent volumes
used in the PersistentVolumeEndpointsTests.


Diffs (updated)
-

  src/tests/persistent_volume_endpoints_tests.cpp 
a57461d881b2bf0175f83b50b0a46167acd5bd3e 

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


Testing
---

`make check` was used to test on OSX.


Thanks,

Greg Mann

Re: Review Request 47521: Added creator principal to '/create-volumes' tests.

2016-05-26 Thread Till Toenshoff 

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


Ship it!




This ship-it is barring your comments. Thanks so much Greg for taking the time 
and typing in the comments as discussed verbally - much appreciated :).

- Till Toenshoff


On May 18, 2016, 8:05 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47521/
> ---
> 
> (Updated May 18, 2016, 8:05 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A creator principal is added to the persistent volumes
> used in the PersistentVolumeEndpointsTests.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> a57461d881b2bf0175f83b50b0a46167acd5bd3e 
> 
> Diff: https://reviews.apache.org/r/47521/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47521: Added creator principal to '/create-volumes' tests.

2016-05-26 Thread Greg Mann 

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




src/tests/persistent_volume_endpoints_tests.cpp (line 32)


add stout/none.hpp



src/tests/persistent_volume_endpoints_tests.cpp (line 1596)


perhaps use a variable for the principal in a subsequent patch?


- Greg Mann


On May 18, 2016, 8:05 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47521/
> ---
> 
> (Updated May 18, 2016, 8:05 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A creator principal is added to the persistent volumes
> used in the PersistentVolumeEndpointsTests.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> a57461d881b2bf0175f83b50b0a46167acd5bd3e 
> 
> Diff: https://reviews.apache.org/r/47521/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 47521: Added creator principal to '/create-volumes' tests.

2016-05-18 Thread Greg Mann 

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

Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.


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


Repository: mesos


Description
---

A creator principal is added to the persistent volumes
used in the PersistentVolumeEndpointsTests.


Diffs
-

  src/tests/persistent_volume_endpoints_tests.cpp 
a57461d881b2bf0175f83b50b0a46167acd5bd3e 

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


Testing
---

`make check` was used to test on OSX.


Thanks,

Greg Mann