Re: Review Request 47515: Enforced a constraint on `DiskInfo.Persistence.principal`.

[Greg Mann]

> On May 26, 2016, 9:12 p.m., Greg Mann wrote:
> > src/master/master.cpp, line 3742
> > 
> >
> > What about just `None()` here?

We actually can't use `None()` here, since the second operand of the `?` 
operator is a `std::string`. One of the second and third operands must be 
convertible into the other, so we use an `Option` here.


- Greg


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


On May 27, 2016, 4:32 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47515/
> ---
> 
> (Updated May 27, 2016, 4:32 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
> ---
> 
> This patch enforces the constraint that the principal
> found in `DiskInfo.Persistence` should equal that of
> the framework or operator responsible for creating
> the volume.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b36b439a1fa07c52146deff2b90728f92676ade3 
>   src/master/master.cpp 0005a29caabcc6a3776037cf86a2b12660e6377b 
>   src/master/validation.hpp f29f9753c75e5abc3402ed23381edcea26517ab3 
>   src/master/validation.cpp f490b899758bdac9676a6f6939918efa6ac52781 
> 
> Diff: https://reviews.apache.org/r/47515/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47515: Enforced a constraint on `DiskInfo.Persistence.principal`.

[Greg Mann]

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

(Updated May 27, 2016, 4:32 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
---

This patch enforces the constraint that the principal
found in `DiskInfo.Persistence` should equal that of
the framework or operator responsible for creating
the volume.


Diffs (updated)
-

  src/master/http.cpp b36b439a1fa07c52146deff2b90728f92676ade3 
  src/master/master.cpp 0005a29caabcc6a3776037cf86a2b12660e6377b 
  src/master/validation.hpp f29f9753c75e5abc3402ed23381edcea26517ab3 
  src/master/validation.cpp f490b899758bdac9676a6f6939918efa6ac52781 

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


Testing
---

`make check` was used to test on OSX at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 47515: Enforced a constraint on `DiskInfo.Persistence.principal`.

[Greg Mann]

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




src/master/http.cpp (line 50)


add stout/option.hpp and stout/none.hpp?


- Greg Mann


On May 23, 2016, 4:37 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47515/
> ---
> 
> (Updated May 23, 2016, 4: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
> ---
> 
> This patch enforces the constraint that the principal
> found in `DiskInfo.Persistence` should equal that of
> the framework or operator responsible for creating
> the volume.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b36b439a1fa07c52146deff2b90728f92676ade3 
>   src/master/master.cpp 0c05938de3e4eaeea2187559e81559f85318fa73 
>   src/master/validation.hpp f29f9753c75e5abc3402ed23381edcea26517ab3 
>   src/master/validation.cpp f490b899758bdac9676a6f6939918efa6ac52781 
> 
> Diff: https://reviews.apache.org/r/47515/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47515: Enforced a constraint on `DiskInfo.Persistence.principal`.

[Till Toenshoff]

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


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 23, 2016, 4:37 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47515/
> ---
> 
> (Updated May 23, 2016, 4: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
> ---
> 
> This patch enforces the constraint that the principal
> found in `DiskInfo.Persistence` should equal that of
> the framework or operator responsible for creating
> the volume.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b36b439a1fa07c52146deff2b90728f92676ade3 
>   src/master/master.cpp 0c05938de3e4eaeea2187559e81559f85318fa73 
>   src/master/validation.hpp f29f9753c75e5abc3402ed23381edcea26517ab3 
>   src/master/validation.cpp f490b899758bdac9676a6f6939918efa6ac52781 
> 
> Diff: https://reviews.apache.org/r/47515/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47515: Enforced a constraint on `DiskInfo.Persistence.principal`.

[Greg Mann]

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




src/master/master.cpp (line 3742)


What about just `None()` here?



src/master/validation.cpp (line 823)


Drop the "A" at the beginning?



src/master/validation.cpp (line 824)


s/request/operation/

and below



src/master/validation.cpp (line 825)


Either remove backticks or replace with single quotes?


- Greg Mann


On May 23, 2016, 4:37 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47515/
> ---
> 
> (Updated May 23, 2016, 4: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
> ---
> 
> This patch enforces the constraint that the principal
> found in `DiskInfo.Persistence` should equal that of
> the framework or operator responsible for creating
> the volume.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b36b439a1fa07c52146deff2b90728f92676ade3 
>   src/master/master.cpp 0c05938de3e4eaeea2187559e81559f85318fa73 
>   src/master/validation.hpp f29f9753c75e5abc3402ed23381edcea26517ab3 
>   src/master/validation.cpp f490b899758bdac9676a6f6939918efa6ac52781 
> 
> Diff: https://reviews.apache.org/r/47515/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47515: Enforced a constraint on `DiskInfo.Persistence.principal`.

[Bernd Mathiske]

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


Ship it!




Ship It!

- Bernd Mathiske


On May 22, 2016, 9:37 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47515/
> ---
> 
> (Updated May 22, 2016, 9: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
> ---
> 
> This patch enforces the constraint that the principal
> found in `DiskInfo.Persistence` should equal that of
> the framework or operator responsible for creating
> the volume.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b36b439a1fa07c52146deff2b90728f92676ade3 
>   src/master/master.cpp 0c05938de3e4eaeea2187559e81559f85318fa73 
>   src/master/validation.hpp f29f9753c75e5abc3402ed23381edcea26517ab3 
>   src/master/validation.cpp f490b899758bdac9676a6f6939918efa6ac52781 
> 
> Diff: https://reviews.apache.org/r/47515/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47515: Enforced a constraint on `DiskInfo.Persistence.principal`.

[Greg Mann]

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

(Updated May 23, 2016, 4: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
---

This patch enforces the constraint that the principal
found in `DiskInfo.Persistence` should equal that of
the framework or operator responsible for creating
the volume.


Diffs (updated)
-

  src/master/http.cpp b36b439a1fa07c52146deff2b90728f92676ade3 
  src/master/master.cpp 0c05938de3e4eaeea2187559e81559f85318fa73 
  src/master/validation.hpp f29f9753c75e5abc3402ed23381edcea26517ab3 
  src/master/validation.cpp f490b899758bdac9676a6f6939918efa6ac52781 

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


Testing
---

`make check` was used to test on OSX at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 47515: Enforced a constraint on `DiskInfo.Persistence.principal`.

[Bernd Mathiske]

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




src/master/master.cpp (line 3750)


Although there does not seem to be a rule in the style guide for this, I'd 
suggest:
{code}
  Option principal = framework->info.has_principal() ? 
framework->info.principal() :
Option::none();
{code}



src/master/validation.cpp (line 823)


was -> has been

same below


- Bernd Mathiske


On May 18, 2016, 1:01 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47515/
> ---
> 
> (Updated May 18, 2016, 1:01 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
> ---
> 
> This patch enforces the constraint that the principal
> found in `DiskInfo.Persistence` should equal that of
> the framework or operator responsible for creating
> the volume.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp c4ca343baa318fbb05ba2cbc3ed892c16478dbcc 
>   src/master/master.cpp b8c732a6178777544f0d09708177d9c68ab0532b 
>   src/master/validation.hpp f29f9753c75e5abc3402ed23381edcea26517ab3 
>   src/master/validation.cpp f490b899758bdac9676a6f6939918efa6ac52781 
> 
> Diff: https://reviews.apache.org/r/47515/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>