You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Aditi Dixit <ad...@gmail.com> on 2015/08/15 10:03:21 UTC
Review Request 37500: Update the FrameworkInfo.user on scheduler
failover
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37500/
-----------------------------------------------------------
Review request for mesos and Vinod Kone.
Bugs: MESOS-3240
https://issues.apache.org/jira/browse/MESOS-3240
Repository: mesos
Description
-------
Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
Diffs
-----
src/master/http.cpp 4dcbc0f3e22f6bdb049da41c950bfc2fbfb89ffa
src/master/master.hpp b288b8a7ad84e49b3ca43966a4d20a64985aa98e
src/master/master.cpp 0330f94ad6fe6ac23061e5f45612133747b39a80
src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c
src/slave/slave.hpp 41d09497be313819a9c78361b8595f6f26dc8460
src/slave/slave.cpp 9061e671bea2d52e8c009e92b3d0a4473dca0ad9
src/tests/master_tests.cpp 0c8e8be2965de3613761515db8a31d72bad97332
Diff: https://reviews.apache.org/r/37500/diff/
Testing
-------
make check
Thanks,
Aditi Dixit
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler
failover
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37500/#review95510
-----------------------------------------------------------
Patch looks great!
Reviews applied: [37500]
All tests passed.
- Mesos ReviewBot
On Aug. 15, 2015, 8:03 a.m., Aditi Dixit wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37500/
> -----------------------------------------------------------
>
> (Updated Aug. 15, 2015, 8:03 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-3240
> https://issues.apache.org/jira/browse/MESOS-3240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
>
>
> Diffs
> -----
>
> src/master/http.cpp 4dcbc0f3e22f6bdb049da41c950bfc2fbfb89ffa
> src/master/master.hpp b288b8a7ad84e49b3ca43966a4d20a64985aa98e
> src/master/master.cpp 0330f94ad6fe6ac23061e5f45612133747b39a80
> src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c
> src/slave/slave.hpp 41d09497be313819a9c78361b8595f6f26dc8460
> src/slave/slave.cpp 9061e671bea2d52e8c009e92b3d0a4473dca0ad9
> src/tests/master_tests.cpp 0c8e8be2965de3613761515db8a31d72bad97332
>
> Diff: https://reviews.apache.org/r/37500/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Aditi Dixit
>
>
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler
failover
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37500/#review96412
-----------------------------------------------------------
Bad patch!
Reviews applied: [37500]
Failed command: ./support/apply-review.sh -n -r 37500
Error:
2015-08-25 20:17:03 URL:https://reviews.apache.org/r/37500/diff/raw/ [11293/11293] -> "37500.patch" [1]
37500.patch:163: trailing whitespace.
warning: 1 line adds whitespace errors.
Successfully applied: Update the FrameworkInfo.user on scheduler failover
Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
Review: https://reviews.apache.org/r/37500
src/tests/fault_tolerance_tests.cpp:1943: trailing whitespace.
+
Failed to commit patch
- Mesos ReviewBot
On Aug. 25, 2015, 8:19 p.m., Aditi Dixit wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37500/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2015, 8:19 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-3240
> https://issues.apache.org/jira/browse/MESOS-3240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
>
>
> Diffs
> -----
>
> src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44
> src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc
> src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf
> src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c
> src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39
> src/slave/slave.cpp 2a99abc00c525a93508d38e74d351d2f36572d86
> src/tests/fault_tolerance_tests.cpp 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075
> src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1
> src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc
> src/tests/slave_tests.cpp d55e9dd4f4eb84a8fda85439e31a38e70890b377
>
> Diff: https://reviews.apache.org/r/37500/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Aditi Dixit
>
>
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler
failover
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37500/#review96422
-----------------------------------------------------------
Patch looks great!
Reviews applied: [37500]
All tests passed.
- Mesos ReviewBot
On Aug. 25, 2015, 8:25 p.m., Aditi Dixit wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37500/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2015, 8:25 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-3240
> https://issues.apache.org/jira/browse/MESOS-3240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
>
>
> Diffs
> -----
>
> src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44
> src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc
> src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf
> src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c
> src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39
> src/slave/slave.cpp 2a99abc00c525a93508d38e74d351d2f36572d86
> src/tests/fault_tolerance_tests.cpp 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075
> src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1
> src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc
> src/tests/slave_tests.cpp d55e9dd4f4eb84a8fda85439e31a38e70890b377
>
> Diff: https://reviews.apache.org/r/37500/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Aditi Dixit
>
>
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler
failover
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37500/#review97456
-----------------------------------------------------------
src/slave/slave.cpp (line 2133)
<https://reviews.apache.org/r/37500/#comment153333>
This message needs to be updated as u are not only updating pid but also user name
- Guangya Liu
On 八月 25, 2015, 8:25 p.m., Aditi Dixit wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37500/
> -----------------------------------------------------------
>
> (Updated 八月 25, 2015, 8:25 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-3240
> https://issues.apache.org/jira/browse/MESOS-3240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
>
>
> Diffs
> -----
>
> src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44
> src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc
> src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf
> src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c
> src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39
> src/slave/slave.cpp 2a99abc00c525a93508d38e74d351d2f36572d86
> src/tests/fault_tolerance_tests.cpp 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075
> src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1
> src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc
> src/tests/slave_tests.cpp d55e9dd4f4eb84a8fda85439e31a38e70890b377
>
> Diff: https://reviews.apache.org/r/37500/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Aditi Dixit
>
>
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler
failover
Posted by Guangya Liu <li...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37500/#review96478
-----------------------------------------------------------
src/master/master.cpp (line 1920)
<https://reviews.apache.org/r/37500/#comment151874>
I think that the logic in
void Master::_subscribe(
const UPID& from,
const scheduler::Call::Subscribe& subscribe,
const Future<bool>& authorized)
should also be udpated?
- Guangya Liu
On Aug. 25, 2015, 8:25 p.m., Aditi Dixit wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37500/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2015, 8:25 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-3240
> https://issues.apache.org/jira/browse/MESOS-3240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
>
>
> Diffs
> -----
>
> src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44
> src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc
> src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf
> src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c
> src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39
> src/slave/slave.cpp 2a99abc00c525a93508d38e74d351d2f36572d86
> src/tests/fault_tolerance_tests.cpp 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075
> src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1
> src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc
> src/tests/slave_tests.cpp d55e9dd4f4eb84a8fda85439e31a38e70890b377
>
> Diff: https://reviews.apache.org/r/37500/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Aditi Dixit
>
>
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler
failover
Posted by Aditi Dixit <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37500/
-----------------------------------------------------------
(Updated Aug. 25, 2015, 8:25 p.m.)
Review request for mesos and Vinod Kone.
Bugs: MESOS-3240
https://issues.apache.org/jira/browse/MESOS-3240
Repository: mesos
Description
-------
Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
Diffs (updated)
-----
src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44
src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc
src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf
src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c
src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39
src/slave/slave.cpp 2a99abc00c525a93508d38e74d351d2f36572d86
src/tests/fault_tolerance_tests.cpp 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075
src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1
src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc
src/tests/slave_tests.cpp d55e9dd4f4eb84a8fda85439e31a38e70890b377
Diff: https://reviews.apache.org/r/37500/diff/
Testing
-------
make check
Thanks,
Aditi Dixit
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler
failover
Posted by Aditi Dixit <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37500/
-----------------------------------------------------------
(Updated Aug. 25, 2015, 8:19 p.m.)
Review request for mesos and Vinod Kone.
Bugs: MESOS-3240
https://issues.apache.org/jira/browse/MESOS-3240
Repository: mesos
Description
-------
Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
Diffs (updated)
-----
src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44
src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc
src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf
src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c
src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39
src/slave/slave.cpp 2a99abc00c525a93508d38e74d351d2f36572d86
src/tests/fault_tolerance_tests.cpp 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075
src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1
src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc
src/tests/slave_tests.cpp d55e9dd4f4eb84a8fda85439e31a38e70890b377
Diff: https://reviews.apache.org/r/37500/diff/
Testing
-------
make check
Thanks,
Aditi Dixit
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler
failover
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37500/#review96407
-----------------------------------------------------------
Bad patch!
Reviews applied: [37500]
Failed command: ./support/apply-review.sh -n -r 37500
Error:
2015-08-25 19:58:12 URL:https://reviews.apache.org/r/37500/diff/raw/ [11351/11351] -> "37500.patch" [1]
error: patch failed: src/tests/fault_tolerance_tests.cpp:1831
error: src/tests/fault_tolerance_tests.cpp: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On Aug. 25, 2015, 7:45 p.m., Aditi Dixit wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37500/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2015, 7:45 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-3240
> https://issues.apache.org/jira/browse/MESOS-3240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
>
>
> Diffs
> -----
>
> src/master/http.cpp 4dcbc0f3e22f6bdb049da41c950bfc2fbfb89ffa
> src/master/master.hpp b288b8a7ad84e49b3ca43966a4d20a64985aa98e
> src/master/master.cpp 0330f94ad6fe6ac23061e5f45612133747b39a80
> src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c
> src/slave/slave.hpp 41d09497be313819a9c78361b8595f6f26dc8460
> src/slave/slave.cpp 9061e671bea2d52e8c009e92b3d0a4473dca0ad9
> src/tests/fault_tolerance_tests.cpp c63599ad36d883d25a23f5bab1929c4e2dad4960
> src/tests/master_tests.cpp 0c8e8be2965de3613761515db8a31d72bad97332
> src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc
> src/tests/slave_tests.cpp cb5a01ed771e66d75091ca33523dbe673e16a86e
>
> Diff: https://reviews.apache.org/r/37500/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Aditi Dixit
>
>
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler
failover
Posted by Aditi Dixit <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37500/
-----------------------------------------------------------
(Updated Aug. 25, 2015, 7:45 p.m.)
Review request for mesos and Vinod Kone.
Changes
-------
1. Shifted the slave updation of FrameworkInfo.user to use the existing UpdateFrameworkMessage instead of creating a new message.
2. Also added test for checking slave updation of FrameworkInfo.user in case of framework reregistration.
3. Added user checking in UpdateFrameworkInfoOnSchedulerFailover.
Bugs: MESOS-3240
https://issues.apache.org/jira/browse/MESOS-3240
Repository: mesos
Description
-------
Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
Diffs (updated)
-----
src/master/http.cpp 4dcbc0f3e22f6bdb049da41c950bfc2fbfb89ffa
src/master/master.hpp b288b8a7ad84e49b3ca43966a4d20a64985aa98e
src/master/master.cpp 0330f94ad6fe6ac23061e5f45612133747b39a80
src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c
src/slave/slave.hpp 41d09497be313819a9c78361b8595f6f26dc8460
src/slave/slave.cpp 9061e671bea2d52e8c009e92b3d0a4473dca0ad9
src/tests/fault_tolerance_tests.cpp c63599ad36d883d25a23f5bab1929c4e2dad4960
src/tests/master_tests.cpp 0c8e8be2965de3613761515db8a31d72bad97332
src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc
src/tests/slave_tests.cpp cb5a01ed771e66d75091ca33523dbe673e16a86e
Diff: https://reviews.apache.org/r/37500/diff/
Testing
-------
make check
Thanks,
Aditi Dixit
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler
failover
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37500/#review96272
-----------------------------------------------------------
src/messages/messages.proto (line 350)
<https://reviews.apache.org/r/37500/#comment151607>
It is better to add some comments here.
i.e.
This message is sent to the slave to update framework user info for a slave.
src/slave/slave.cpp (line 2173)
<https://reviews.apache.org/r/37500/#comment151608>
Update the indent here
- Guangya Liu
On Aug. 15, 2015, 8:03 a.m., Aditi Dixit wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37500/
> -----------------------------------------------------------
>
> (Updated Aug. 15, 2015, 8:03 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-3240
> https://issues.apache.org/jira/browse/MESOS-3240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
>
>
> Diffs
> -----
>
> src/master/http.cpp 4dcbc0f3e22f6bdb049da41c950bfc2fbfb89ffa
> src/master/master.hpp b288b8a7ad84e49b3ca43966a4d20a64985aa98e
> src/master/master.cpp 0330f94ad6fe6ac23061e5f45612133747b39a80
> src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c
> src/slave/slave.hpp 41d09497be313819a9c78361b8595f6f26dc8460
> src/slave/slave.cpp 9061e671bea2d52e8c009e92b3d0a4473dca0ad9
> src/tests/master_tests.cpp 0c8e8be2965de3613761515db8a31d72bad97332
>
> Diff: https://reviews.apache.org/r/37500/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Aditi Dixit
>
>
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler
failover
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Aug. 24, 2015, 4:56 p.m., Vinod Kone wrote:
> > This review is a bit hard to follow because it's doing multiple things. I would recommend you to split this into multiple reviews
> >
> > #1) Expose framework user in state.json
> > #2) Update framework user on re-registration (need a test for this!)
>
> Aditi Dixit wrote:
> Added the test. How do I split this into multiple reviews? Discard this one and open two new ones by shifting the code into new branches and then opening review requests?
You could:
```
# To update this review into the first in a review chain:
git branch temp # Save your work.
<Delete part of your changes> # i.e. the re-registration stuff.
git add --update
git commit --amend
# For the second review:
git checkout temp -- . # Restore everything you deleted into a second commit.
git commit
support/post-reviews.py
```
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37500/#review96240
-----------------------------------------------------------
On Aug. 25, 2015, 1:25 p.m., Aditi Dixit wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37500/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2015, 1:25 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-3240
> https://issues.apache.org/jira/browse/MESOS-3240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
>
>
> Diffs
> -----
>
> src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44
> src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc
> src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf
> src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c
> src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39
> src/slave/slave.cpp 2a99abc00c525a93508d38e74d351d2f36572d86
> src/tests/fault_tolerance_tests.cpp 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075
> src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1
> src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc
> src/tests/slave_tests.cpp d55e9dd4f4eb84a8fda85439e31a38e70890b377
>
> Diff: https://reviews.apache.org/r/37500/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Aditi Dixit
>
>
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler
failover
Posted by Vinod Kone <vi...@gmail.com>.
> On Aug. 24, 2015, 11:56 p.m., Vinod Kone wrote:
> > This review is a bit hard to follow because it's doing multiple things. I would recommend you to split this into multiple reviews
> >
> > #1) Expose framework user in state.json
> > #2) Update framework user on re-registration (need a test for this!)
>
> Aditi Dixit wrote:
> Added the test. How do I split this into multiple reviews? Discard this one and open two new ones by shifting the code into new branches and then opening review requests?
>
> Joseph Wu wrote:
> You could:
> ```
> # To update this review into the first in a review chain:
> git branch temp # Save your work.
> <Delete part of your changes> # i.e. the re-registration stuff.
> git add --update
> git commit --amend
>
> # For the second review:
> git checkout temp -- . # Restore everything you deleted into a second commit.
> git commit
>
> support/post-reviews.py
> ```
Thanks Joseph for the workflow tip. I typically do splits a little differently.
$ git reset HEAD~
$ git add -p # Selectively add chunks of a file(s) to the index!
$ git commit -m "First review" # Make sure to add the "Review: <blah>" line if you want the current review to be updated
$ git add -a # Add the rest of the changes to the index
$ git commit -m "Second review"
I'll take a look once you split and update.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37500/#review96240
-----------------------------------------------------------
On Aug. 25, 2015, 8:25 p.m., Aditi Dixit wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37500/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2015, 8:25 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-3240
> https://issues.apache.org/jira/browse/MESOS-3240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
>
>
> Diffs
> -----
>
> src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44
> src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc
> src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf
> src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c
> src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39
> src/slave/slave.cpp 2a99abc00c525a93508d38e74d351d2f36572d86
> src/tests/fault_tolerance_tests.cpp 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075
> src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1
> src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc
> src/tests/slave_tests.cpp d55e9dd4f4eb84a8fda85439e31a38e70890b377
>
> Diff: https://reviews.apache.org/r/37500/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Aditi Dixit
>
>
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler
failover
Posted by Aditi Dixit <ad...@gmail.com>.
> On Aug. 24, 2015, 11:56 p.m., Vinod Kone wrote:
> > This review is a bit hard to follow because it's doing multiple things. I would recommend you to split this into multiple reviews
> >
> > #1) Expose framework user in state.json
> > #2) Update framework user on re-registration (need a test for this!)
Added the test. How do I split this into multiple reviews? Discard this one and open two new ones by shifting the code into new branches and then opening review requests?
- Aditi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37500/#review96240
-----------------------------------------------------------
On Aug. 25, 2015, 7:45 p.m., Aditi Dixit wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37500/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2015, 7:45 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-3240
> https://issues.apache.org/jira/browse/MESOS-3240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
>
>
> Diffs
> -----
>
> src/master/http.cpp 4dcbc0f3e22f6bdb049da41c950bfc2fbfb89ffa
> src/master/master.hpp b288b8a7ad84e49b3ca43966a4d20a64985aa98e
> src/master/master.cpp 0330f94ad6fe6ac23061e5f45612133747b39a80
> src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c
> src/slave/slave.hpp 41d09497be313819a9c78361b8595f6f26dc8460
> src/slave/slave.cpp 9061e671bea2d52e8c009e92b3d0a4473dca0ad9
> src/tests/fault_tolerance_tests.cpp c63599ad36d883d25a23f5bab1929c4e2dad4960
> src/tests/master_tests.cpp 0c8e8be2965de3613761515db8a31d72bad97332
> src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc
> src/tests/slave_tests.cpp cb5a01ed771e66d75091ca33523dbe673e16a86e
>
> Diff: https://reviews.apache.org/r/37500/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Aditi Dixit
>
>
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler
failover
Posted by Aditi Dixit <ad...@gmail.com>.
> On Aug. 24, 2015, 11:56 p.m., Vinod Kone wrote:
> > src/master/master.cpp, lines 1879-1886
> > <https://reviews.apache.org/r/37500/diff/1/?file=1041022#file1041022line1879>
> >
> > Per the design doc, we don't need a new message. You should be able to augment the existing UpdateFrameworkMessage to include FrameworkInfo.
> >
> > Any particular reason you introduced a new one?
Well, I was a bit reluctant on merging logic of both the PID updation and user updation into a single handler and wanted separation of concerns. Basically I am not confident that adding the user logic and modifying the message won't introduce bugs for the PID updation part. Please review with that in mind
> On Aug. 24, 2015, 11:56 p.m., Vinod Kone wrote:
> > src/master/master.hpp, lines 1475-1479
> > <https://reviews.apache.org/r/37500/diff/1/?file=1041021#file1041021line1475>
> >
> > So you are not updating the 'info.user' at all? How come?
My bad. Added a test to catch this as well.
- Aditi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37500/#review96240
-----------------------------------------------------------
On Aug. 25, 2015, 7:45 p.m., Aditi Dixit wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37500/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2015, 7:45 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-3240
> https://issues.apache.org/jira/browse/MESOS-3240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
>
>
> Diffs
> -----
>
> src/master/http.cpp 4dcbc0f3e22f6bdb049da41c950bfc2fbfb89ffa
> src/master/master.hpp b288b8a7ad84e49b3ca43966a4d20a64985aa98e
> src/master/master.cpp 0330f94ad6fe6ac23061e5f45612133747b39a80
> src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c
> src/slave/slave.hpp 41d09497be313819a9c78361b8595f6f26dc8460
> src/slave/slave.cpp 9061e671bea2d52e8c009e92b3d0a4473dca0ad9
> src/tests/fault_tolerance_tests.cpp c63599ad36d883d25a23f5bab1929c4e2dad4960
> src/tests/master_tests.cpp 0c8e8be2965de3613761515db8a31d72bad97332
> src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc
> src/tests/slave_tests.cpp cb5a01ed771e66d75091ca33523dbe673e16a86e
>
> Diff: https://reviews.apache.org/r/37500/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Aditi Dixit
>
>
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler
failover
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37500/#review96240
-----------------------------------------------------------
This review is a bit hard to follow because it's doing multiple things. I would recommend you to split this into multiple reviews
#1) Expose framework user in state.json
#2) Update framework user on re-registration (need a test for this!)
src/master/master.hpp
<https://reviews.apache.org/r/37500/#comment151577>
So you are not updating the 'info.user' at all? How come?
src/master/master.cpp (lines 1879 - 1886)
<https://reviews.apache.org/r/37500/#comment151579>
Per the design doc, we don't need a new message. You should be able to augment the existing UpdateFrameworkMessage to include FrameworkInfo.
Any particular reason you introduced a new one?
src/slave/slave.cpp (lines 446 - 450)
<https://reviews.apache.org/r/37500/#comment151581>
no need for this if you just use the existing handler for update framework message.
src/tests/master_tests.cpp (line 2973)
<https://reviews.apache.org/r/37500/#comment151582>
s/FrameworkWebUIUrlCapabilitiesAndUser/FrameworkState/
- Vinod Kone
On Aug. 15, 2015, 8:03 a.m., Aditi Dixit wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37500/
> -----------------------------------------------------------
>
> (Updated Aug. 15, 2015, 8:03 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-3240
> https://issues.apache.org/jira/browse/MESOS-3240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too.
>
>
> Diffs
> -----
>
> src/master/http.cpp 4dcbc0f3e22f6bdb049da41c950bfc2fbfb89ffa
> src/master/master.hpp b288b8a7ad84e49b3ca43966a4d20a64985aa98e
> src/master/master.cpp 0330f94ad6fe6ac23061e5f45612133747b39a80
> src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c
> src/slave/slave.hpp 41d09497be313819a9c78361b8595f6f26dc8460
> src/slave/slave.cpp 9061e671bea2d52e8c009e92b3d0a4473dca0ad9
> src/tests/master_tests.cpp 0c8e8be2965de3613761515db8a31d72bad97332
>
> Diff: https://reviews.apache.org/r/37500/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Aditi Dixit
>
>