Re: Review Request 39098: Changed secret field in Credential from 'bytes' to 'string'

2015-10-13 Thread Isabel Jimenez

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

(Updated Oct. 13, 2015, 7:53 a.m.)


Review request for mesos and Michael Park.


Changes
---

+ test


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


Repository: mesos


Description
---

When decoding the JSON credential file into the Credential protobuf object, the 
'secret' which is in 'bytes' is mapped into base64 string automatically by 
protobuf from JSON. This creates an unintended behavior, forcing users to 
encode in base64 their secret when wanting to pass a JSON file to the 
--credentials flag.


Diffs (updated)
-

  include/mesos/mesos.proto 4a16be1 
  src/examples/java/TestExceptionFramework.java 78720b0 
  src/examples/java/TestFramework.java aad94c0 
  src/tests/credentials_tests.cpp ced27c4 

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


Testing
---

make check


Thanks,

Isabel Jimenez



Re: Review Request 39098: Changed secret field in Credential from 'bytes' to 'string'

2015-10-13 Thread Guangya Liu

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



src/tests/credentials_tests.cpp (line 116)


Can you pls add some comments here for this unit test?


- Guangya Liu


On 十月 13, 2015, 7:53 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39098/
> ---
> 
> (Updated 十月 13, 2015, 7:53 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3560
> https://issues.apache.org/jira/browse/MESOS-3560
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When decoding the JSON credential file into the Credential protobuf object, 
> the 'secret' which is in 'bytes' is mapped into base64 string automatically 
> by protobuf from JSON. This creates an unintended behavior, forcing users to 
> encode in base64 their secret when wanting to pass a JSON file to the 
> --credentials flag.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 4a16be1 
>   src/examples/java/TestExceptionFramework.java 78720b0 
>   src/examples/java/TestFramework.java aad94c0 
>   src/tests/credentials_tests.cpp ced27c4 
> 
> Diff: https://reviews.apache.org/r/39098/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 39098: Changed secret field in Credential from 'bytes' to 'string'

2015-10-13 Thread Isabel Jimenez

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

(Updated Oct. 13, 2015, 8:45 a.m.)


Review request for mesos and Michael Park.


Changes
---

review comments


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


Repository: mesos


Description
---

When decoding the JSON credential file into the Credential protobuf object, the 
'secret' which is in 'bytes' is mapped into base64 string automatically by 
protobuf from JSON. This creates an unintended behavior, forcing users to 
encode in base64 their secret when wanting to pass a JSON file to the 
--credentials flag.


Diffs (updated)
-

  include/mesos/mesos.proto 4a16be1 
  src/examples/java/TestExceptionFramework.java 78720b0 
  src/examples/java/TestFramework.java aad94c0 
  src/tests/credentials_tests.cpp ced27c4 

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


Testing
---

make check


Thanks,

Isabel Jimenez



Re: Review Request 39098: Changed secret field in Credential from 'bytes' to 'string'

2015-10-13 Thread Michael Park

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

Ship it!


I added the comments to the test you added, but some of them apply to 
`AuthenticatedSlaveText` as well. Let's fix those as well :)


src/tests/credentials_tests.cpp (line 119)


Let's rename this `s/flags/masterFlags/` and move it to just above 
`flags.load(values, true)` below.



src/tests/credentials_tests.cpp (line 121)


`path::join` returns a temporary string right?
In which case: `s/const string &/string/`.



src/tests/credentials_tests.cpp (line 145)


Add newline after



src/tests/credentials_tests.cpp (line 147)


Let's just use `s/flags.credentials.get().value/path/` here?



src/tests/credentials_tests.cpp (line 158)


Remove newline.


- Michael Park


On Oct. 13, 2015, 7:53 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39098/
> ---
> 
> (Updated Oct. 13, 2015, 7:53 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3560
> https://issues.apache.org/jira/browse/MESOS-3560
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When decoding the JSON credential file into the Credential protobuf object, 
> the 'secret' which is in 'bytes' is mapped into base64 string automatically 
> by protobuf from JSON. This creates an unintended behavior, forcing users to 
> encode in base64 their secret when wanting to pass a JSON file to the 
> --credentials flag.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 4a16be1 
>   src/examples/java/TestExceptionFramework.java 78720b0 
>   src/examples/java/TestFramework.java aad94c0 
>   src/tests/credentials_tests.cpp ced27c4 
> 
> Diff: https://reviews.apache.org/r/39098/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 39098: Changed secret field in Credential from 'bytes' to 'string'

2015-10-13 Thread Michael Park

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

Ship it!


I've committed this with the following minor style fixes!


src/tests/credentials_tests.cpp (line 55)


`s/> >/>>/`



src/tests/credentials_tests.cpp (line 61)


`s/> >/>>/`



src/tests/credentials_tests.cpp (line 85)


`s/const std::string&/std::string/`



src/tests/credentials_tests.cpp (line 86)


newline after this line



src/tests/credentials_tests.cpp (line 88)


3 space indent => 4 space indent



src/tests/credentials_tests.cpp (line 142)


3 space indent => 4 space indent



src/tests/credentials_tests.cpp (lines 146 - 147)


Fits in one line.


- Michael Park


On Oct. 13, 2015, 8:45 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39098/
> ---
> 
> (Updated Oct. 13, 2015, 8:45 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3560
> https://issues.apache.org/jira/browse/MESOS-3560
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When decoding the JSON credential file into the Credential protobuf object, 
> the 'secret' which is in 'bytes' is mapped into base64 string automatically 
> by protobuf from JSON. This creates an unintended behavior, forcing users to 
> encode in base64 their secret when wanting to pass a JSON file to the 
> --credentials flag.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 4a16be1 
>   src/examples/java/TestExceptionFramework.java 78720b0 
>   src/examples/java/TestFramework.java aad94c0 
>   src/tests/credentials_tests.cpp ced27c4 
> 
> Diff: https://reviews.apache.org/r/39098/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 39098: Changed secret field in Credential from 'bytes' to 'string'

2015-10-09 Thread Till Toenshoff

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

Ship it!


Ship It!

- Till Toenshoff


On Oct. 8, 2015, 1 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39098/
> ---
> 
> (Updated Oct. 8, 2015, 1 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3560
> https://issues.apache.org/jira/browse/MESOS-3560
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When decoding the JSON credential file into the Credential protobuf object, 
> the 'secret' which is in 'bytes' is mapped into base64 string automatically 
> by protobuf from JSON. This creates an unintended behavior, forcing users to 
> encode in base64 their secret when wanting to pass a JSON file to the 
> --credentials flag.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 4a16be1 
>   src/examples/java/TestExceptionFramework.java 78720b0 
>   src/examples/java/TestFramework.java aad94c0 
> 
> Diff: https://reviews.apache.org/r/39098/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 39098: Changed secret field in Credential from 'bytes' to 'string'

2015-10-07 Thread Isabel Jimenez

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

(Updated Oct. 8, 2015, 1 a.m.)


Review request for mesos and Michael Park.


Changes
---

Added java framework example changes


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


Repository: mesos


Description
---

When decoding the JSON credential file into the Credential protobuf object, the 
'secret' which is in 'bytes' is mapped into base64 string automatically by 
protobuf from JSON. This creates an unintended behavior, forcing users to 
encode in base64 their secret when wanting to pass a JSON file to the 
--credentials flag.


Diffs (updated)
-

  include/mesos/mesos.proto 4a16be1 
  src/examples/java/TestExceptionFramework.java 78720b0 
  src/examples/java/TestFramework.java aad94c0 

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


Testing
---

make check


Thanks,

Isabel Jimenez