Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 10, 2015, 7:20 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Re-opened to remove stale depedency. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp 53420ca7d503296fbe11b1ea0795afe2ebf86255 src/master/master.cpp d699e4bc3cf734a516a6baf329919e04744b5702 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review94496 --- Ship it! I left some comments here so you could see the issues I noticed as I went over this, mostly around upgrading and downgrading across pid and http. I've applied fixes for all of these to avoid more round-trips and will get this committed shortly, nice work! src/master/master.cpp (line 4950) https://reviews.apache.org/r/36720/#comment149133 Much like when we remove a pid-based framework, we need to wipe the authentication related data here. src/master/master.cpp (lines 4963 - 4968) https://reviews.apache.org/r/36720/#comment149135 Perhaps we should just push the unsetting of the other connection into updateConnection and have one for both http and pid. src/master/master.cpp (line 4967) https://reviews.apache.org/r/36720/#comment149136 We need to remove the old one from `authenticated` and `principals` here when updrading to http, much like we do when a pid based framework is removed. Yes.. this stuff is a nasty mess, we need to make failover as simple as the composition of: disconnect - reconnect! src/master/master.cpp (line 4983) https://reviews.apache.org/r/36720/#comment149129 This check will crash when an http scheduler is downgrading to a pid framework. src/master/master.cpp (lines 5010 - 5015) https://reviews.apache.org/r/36720/#comment149132 This won't work for downgrades from http to pid, since there was no principal in the map yet. src/master/master.cpp (lines 5039 - 5040) https://reviews.apache.org/r/36720/#comment149138 Could we do this before reactivating? Note that your comment seems to hint that it must come after for some reason, but it doesn't need to. - Ben Mahler On Aug. 7, 2015, 2:27 a.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 7, 2015, 2:27 a.m.) Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp 53420ca7d503296fbe11b1ea0795afe2ebf86255 src/master/master.cpp d699e4bc3cf734a516a6baf329919e04744b5702 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
On Aug. 6, 2015, 6:39 p.m., Ben Mahler wrote: src/master/master.cpp, line 4990 https://reviews.apache.org/r/36720/diff/9/?file=1033626#file1033626line4990 Don't we still want this since we call pid.get()? Persisted the CHECK_SOME. Removed the misleading message though as we have now implemented http framework failover. - Anand --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review94428 --- On Aug. 6, 2015, 4:25 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 6, 2015, 4:25 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 7, 2015, 2:27 a.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Modified signature for subscribe(...), added _failoverFramework. Modified some TODO messages to be more explicit. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs (updated) - src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp 53420ca7d503296fbe11b1ea0795afe2ebf86255 src/master/master.cpp d699e4bc3cf734a516a6baf329919e04744b5702 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review94428 --- Thanks, I'll get the HttpConnection change committed, left some notes about things I updated. Also opened some issues in the subscribe path that I noticed while doing this. src/common/http.hpp (line 50) https://reviews.apache.org/r/36720/#comment149022 Hm.. let's at least say that it's based on the content type. src/common/http.cpp (line 49) https://reviews.apache.org/r/36720/#comment149021 No need for std:: here src/common/http.cpp (lines 61 - 63) https://reviews.apache.org/r/36720/#comment149020 How about we just put UNREACHABLE at the bottom outside the switch? That way, we don't lose the switch warning for missing enum cases, right? src/master/master.hpp (lines 1542 - 1544) https://reviews.apache.org/r/36720/#comment149023 Let's have the master decide when to set up this callback. On that note, how about we add a `closed()` on the HttpConnection as well? src/master/master.cpp (line 2216) https://reviews.apache.org/r/36720/#comment149024 This change looks like a regression, now we're sending to '`framework-pid`' instead of '`from`'? src/master/master.cpp https://reviews.apache.org/r/36720/#comment149027 Don't we still want this since we call pid.get()? - Ben Mahler On Aug. 6, 2015, 4:25 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 6, 2015, 4:25 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review94413 --- src/common/http.cpp (line 60) https://reviews.apache.org/r/36720/#comment148999 need a return here to aviod compiler warning. - Vinod Kone On Aug. 6, 2015, 4:08 a.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 6, 2015, 4:08 a.m.) Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
On Aug. 6, 2015, 4:10 p.m., Vinod Kone wrote: src/common/http.cpp, line 60 https://reviews.apache.org/r/36720/diff/8/?file=1033227#file1033227line60 need a return here to aviod compiler warning. I am putting in a fix for that. This looks to be as a GCC bug. With -Werror=uninitialized, -Wswitch turned on in our code. It shouldn't ever complain. Clang rightfully figures this out but GCC does not. Would go ahead and put in a default: UNREACHABLE() workaround here for now. - Anand --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review94413 --- On Aug. 6, 2015, 4:08 a.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 6, 2015, 4:08 a.m.) Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 6, 2015, 4:25 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Added a UNREACHABLE for default case to put off GCC for now. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs (updated) - src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 6, 2015, 4:08 a.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Added a continuation function for failoverFramework(...) that is common to both http and pid frameworks. Made the readerClosed callback as part of addFramework similar to link(...) Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs (updated) - src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 5, 2015, 10:37 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Added a new HttpConnection struct instead of optimistically creating framework objects and deleting them for re-registration. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs (updated) - src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 4, 2015, 10 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Rebased from master from Ben's latest commit. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs (updated) - src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp cd0a5c863d4f74c3bcfd61d8e3046b3ab249be67 src/master/master.cpp 87e11d512f68e2b7285ee1901d422991208baf15 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 4, 2015, 11:57 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Rebased from master after Ben's commit for exited Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs (updated) - src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
On Aug. 4, 2015, 9:07 p.m., Ben Mahler wrote: I'll get the /call validation committed. Let's follow up to get tests added for each of the error cases in /call. - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review94111 --- On Aug. 4, 2015, 10 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 4, 2015, 10 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp cd0a5c863d4f74c3bcfd61d8e3046b3ab249be67 src/master/master.cpp 87e11d512f68e2b7285ee1901d422991208baf15 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review94130 --- I'll get the exited changes committed. src/master/master.cpp (line 969) https://reviews.apache.org/r/36720/#comment148645 Hm.. let's check that the framework ids are equal when the writers match. src/master/master.cpp (line 971) https://reviews.apache.org/r/36720/#comment148644 s/exited/disconnection/ src/master/master.cpp (line 1021) https://reviews.apache.org/r/36720/#comment148648 Let's put `_exited` after both `exited` implementations, since it is a shared continuation. - Ben Mahler On Aug. 4, 2015, 10 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 4, 2015, 10 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp cd0a5c863d4f74c3bcfd61d8e3046b3ab249be67 src/master/master.cpp 87e11d512f68e2b7285ee1901d422991208baf15 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
On July 29, 2015, 7:12 p.m., Ben Mahler wrote: src/master/master.cpp, lines 2128-2159 https://reviews.apache.org/r/36720/diff/3/?file=1021871#file1021871line2128 Any reason you're skipping validation (authorization) of the framework? Looks like we should pull out the pid specific code from validate for now: it looks like we already have to do synchronous checks inside the continuations anyway. For example, `_reregisterFramework` checks to see if it is authenticated, but it is missing the check that it is the right principal! Looks like we should s/validate/authorize/ and pull out an `isAuthenticated` that we can call synchronously as well. Anand Mazumdar wrote: Primarily due to the reasons you had mentioned earlier. There was a lot of pid specific code in validate. Also , call validation in general were supposed to be handled in MESOS-2497, so I refrained from making the changes. Looks like you already did them in r36919, Thanks ! Would take up validation in a different patch as we had discussed earlier. I am dropping the issue for now. Let me know if you think otherwise. - Anand --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review93499 --- On Aug. 4, 2015, 4:33 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 4, 2015, 4:33 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 4, 2015, 4:33 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Added functionality to failover http frameworks. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description (updated) --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs (updated) - src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 Diff: https://reviews.apache.org/r/36720/diff/ Testing (updated) --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review94111 --- I'll get the /call validation committed. src/master/http.cpp (line 355) https://reviews.apache.org/r/36720/#comment148622 Need an else here for UnsupportedMediaType, I'll add this in. - Ben Mahler On Aug. 4, 2015, 4:33 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 4, 2015, 4:33 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
On July 29, 2015, 7:12 p.m., Ben Mahler wrote: src/master/master.cpp, lines 2128-2159 https://reviews.apache.org/r/36720/diff/3/?file=1021871#file1021871line2128 Any reason you're skipping validation (authorization) of the framework? Looks like we should pull out the pid specific code from validate for now: it looks like we already have to do synchronous checks inside the continuations anyway. For example, `_reregisterFramework` checks to see if it is authenticated, but it is missing the check that it is the right principal! Looks like we should s/validate/authorize/ and pull out an `isAuthenticated` that we can call synchronously as well. Primarily due to the reasons you had mentioned earlier. There was a lot of pid specific code in validate. Also , call validation in general were supposed to be handled in MESOS-2497, so I refrained from making the changes. Looks like you already did them in r36919, Thanks ! On July 29, 2015, 7:12 p.m., Ben Mahler wrote: src/master/master.cpp, lines 2162-2193 https://reviews.apache.org/r/36720/diff/3/?file=1021871#file1021871line2162 This was a bit confusing, since it says registering but you're planning to also have it include re-registration? This looks like `_subscribe` given it is the continuation. Yes, I intend to make re-register also call into this function. You are also right about this being the _subscribe. Since I had not yet taken care of validation as per my earlier argument, I went ahead with naming it as subscribe for now. When we have the validation routine , it would just become a continuation function and can be renamed. Also , failover logic would be implemented inside this function and the existing _reregisterFramework(...) would be calling into this function. What do you think ? On July 29, 2015, 7:12 p.m., Ben Mahler wrote: src/master/http.cpp, lines 324-379 https://reviews.apache.org/r/36720/diff/3/?file=1021869#file1021869line324 JSON should be do-able already, here's a suggestion to clean this up a bit (note that it is handling JSON): ``` FutureResponse Master::Http::call(const Request request) const { scheduler::Call call; // TODO(anand): Content type values are case-insensitive. Optionstring contentType = request.headers.get(Content-Type); if (contentType.isNone()) { return BadRequest(Expecting 'Content-Type' to be present); } if (contentType.get() == APPLICATION_PROTOBUF) { if (!call.ParseFromString(request.body)) { return BadRequest(Failed to deserialize body into Call protobuf); } } else if (contentType.get() == APPLICATION_JSON) { TryJSON::Value value = JSON::parse(request.body); if (value.isError()) { return BadRequest(Failed to parse body into JSON: + value.error()); } Tryscheduler::Call parse = ::protobuf::parsescheduler::Call(value.get()); if (parse.isError()) { return BadRequest(Failed to convert JSON into Call protobuf: + parse.error()); } call = parse.get(); } // Default to sending back JSON. ContentType responseContentType = ContentType::JSON; // TODO(anand): Use request.accepts() once available. Optionstring acceptType = request.headers.get(Accept); if (acceptType.get() == APPLICATION_PROTOBUF) { responseContentType = ContentType::PROTOBUF; } switch (call.type()) { case scheduler::Call::SUBSCRIBE: { Pipe pipe; OK ok; ok.type = Response::PIPE; ok.reader = pipe.reader(); master-subscribeHttp( call.subscribe(), responseContentType, pipe.writer()); return ok; } default: break; } return NotImplemented(); } ``` Thanks, I would include the JSON logic. I thought adding the JSON workflow might be more convoluted then this and hence wanted it to be part of a different patch, my bad. - Anand --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review93499 --- On July 25, 2015, 2:32 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated July 25, 2015, 2:32 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2294
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review93499 --- Looks like this needs a rebase? Wasn't able to get it applied without `--3way`. I'm going to pull out the call Call validation to hopefully make this a bit easier. Per my comment below, I'd suggest we try to tackle the 'validate' refactor in an independent patch to pull apart the authorization vs authentication checks. src/master/http.cpp (lines 324 - 379) https://reviews.apache.org/r/36720/#comment147888 JSON should be do-able already, here's a suggestion to clean this up a bit (note that it is handling JSON): ``` FutureResponse Master::Http::call(const Request request) const { scheduler::Call call; // TODO(anand): Content type values are case-insensitive. Optionstring contentType = request.headers.get(Content-Type); if (contentType.isNone()) { return BadRequest(Expecting 'Content-Type' to be present); } if (contentType.get() == APPLICATION_PROTOBUF) { if (!call.ParseFromString(request.body)) { return BadRequest(Failed to deserialize body into Call protobuf); } } else if (contentType.get() == APPLICATION_JSON) { TryJSON::Value value = JSON::parse(request.body); if (value.isError()) { return BadRequest(Failed to parse body into JSON: + value.error()); } Tryscheduler::Call parse = ::protobuf::parsescheduler::Call(value.get()); if (parse.isError()) { return BadRequest(Failed to convert JSON into Call protobuf: + parse.error()); } call = parse.get(); } // Default to sending back JSON. ContentType responseContentType = ContentType::JSON; // TODO(anand): Use request.accepts() once available. Optionstring acceptType = request.headers.get(Accept); if (acceptType.get() == APPLICATION_PROTOBUF) { responseContentType = ContentType::PROTOBUF; } switch (call.type()) { case scheduler::Call::SUBSCRIBE: { Pipe pipe; OK ok; ok.type = Response::PIPE; ok.reader = pipe.reader(); master-subscribeHttp( call.subscribe(), responseContentType, pipe.writer()); return ok; } default: break; } return NotImplemented(); } ``` src/master/master.cpp (lines 2110 - 2141) https://reviews.apache.org/r/36720/#comment147890 Any reason you're skipping validation (authorization) of the framework? Looks like we should pull out the pid specific code from validate for now: it looks like we already have to do synchronous checks inside the continuations anyway. For example, `_reregisterFramework` checks to see if it is authenticated, but it is missing the check that it is the right principal! Looks like we should s/validate/authorize/ and pull out an `isAuthenticated` that we can call synchronously as well. src/master/master.cpp (lines 2144 - 2175) https://reviews.apache.org/r/36720/#comment147889 This was a bit confusing, since it says registering but you're planning to also have it include re-registration? This looks like `_subscribe` given it is the continuation. - Ben Mahler On July 25, 2015, 2:32 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated July 25, 2015, 2:32 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Diffs - src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + test. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
On July 26, 2015, 9:37 p.m., Jojy Varghese wrote: src/master/http.cpp, line 339 https://reviews.apache.org/r/36720/diff/3/?file=1021869#file1021869line339 complexity of a function is measured by the nested if condition in it. Maybe we accomplish the same using simple small functions ? Yes, we intend to move these in the near future. To add a bit of context here, the validation logic would be handled in MESOS-2497. I added the band-aid logic here to prevent the master from crashing for now. Here is the TODO mentioned in the code: // Move this to unmarshal(...) method. - Anand --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review93060 --- On July 25, 2015, 2:32 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated July 25, 2015, 2:32 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Diffs - src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + test. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review93060 --- src/master/http.cpp (line 339) https://reviews.apache.org/r/36720/#comment147323 complexity of a function is measured by the nested if condition in it. Maybe we accomplish the same using simple small functions ? - Jojy Varghese On July 25, 2015, 2:32 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated July 25, 2015, 2:32 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Diffs - src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + test. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review93048 --- src/master/http.cpp (line 359) https://reviews.apache.org/r/36720/#comment147318 Why not static initialization pattern : OK ok = { .type = Response::PIPE, .reader = pipe.reader(), }; Maybe its not allowed in the coding style but I would think it looks elegant. - Jojy Varghese On July 25, 2015, 2:32 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated July 25, 2015, 2:32 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Diffs - src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + test. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated July 25, 2015, 7:26 a.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Diffs - src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + test. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated July 25, 2015, 7:42 a.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Diffs (updated) - src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + test. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review93022 --- Bad patch! Reviews applied: [36733, 36717, 36318] Failed command: ./support/apply-review.sh -n -r 36318 Error: 2015-07-25 07:28:15 URL:https://reviews.apache.org/r/36318/diff/raw/ [23397/23397] - 36318.patch [1] error: patch failed: src/master/master.hpp:89 error: src/master/master.hpp: patch does not apply Failed to apply patch - Mesos ReviewBot On July 25, 2015, 7:26 a.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated July 25, 2015, 7:26 a.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Diffs - src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + test. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated July 25, 2015, 2:32 p.m.) Review request for mesos and Ben Mahler. Changes --- *Trigger reviewbot again* Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Diffs - src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + test. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review93033 --- Patch looks great! Reviews applied: [36733, 36717, 36318, 36720] All tests passed. - Mesos ReviewBot On July 25, 2015, 2:32 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated July 25, 2015, 2:32 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Diffs - src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + test. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated July 25, 2015, 7 a.m.) Review request for mesos and Ben Mahler. Changes --- Moved the tests to use the decoder from MESOS-3067. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Diffs (updated) - src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + test. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review93021 --- Bad patch! Reviews applied: [36733] Failed command: ./support/apply-review.sh -n -r 36733 Error: 2015-07-25 07:07:12 URL:https://reviews.apache.org/r/36733/diff/raw/ [58421/58421] - 36733.patch [1] error: patch failed: src/master/master.hpp:901 error: src/master/master.hpp: patch does not apply Failed to apply patch - Mesos ReviewBot On July 25, 2015, 7 a.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated July 25, 2015, 7 a.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Diffs - src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + test. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review92728 --- Bad patch! Reviews applied: [36717, 36318] Failed command: ./support/apply-review.sh -n -r 36318 Error: 2015-07-23 06:05:14 URL:https://reviews.apache.org/r/36318/diff/raw/ [29019/29019] - 36318.patch [1] error: patch failed: src/master/master.hpp:1512 error: src/master/master.hpp: patch does not apply Failed to apply patch - Mesos ReviewBot On July 23, 2015, 4:50 a.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated July 23, 2015, 4:50 a.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Diffs - src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2 src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + test. Thanks, Anand Mazumdar
Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- Review request for mesos and Ben Mahler. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Diffs - src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2 src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + test. Thanks, Anand Mazumdar