This is an automated email from the ASF dual-hosted git repository.

alexr pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 4930ec2e141920411fb9050500f385f5ef6a78a2
Author: Benno Evers <[email protected]>
AuthorDate: Tue Aug 28 21:26:36 2018 +0200

    Cleaned up some style issues in `ReadOnlyHandler`.
    
    This commit fixes several minor style issues:
     - Sorted member function declarations of `ReadOnlyHandler`
       alphabetically.
     - Added notes to remind readers of the fact that requests
       to certain endpoints are batched.
     - Changed captured variable in `/frameworks` endpoint handler.
    
    Review: https://reviews.apache.org/r/68537/
---
 src/common/http.hpp             |  2 +-
 src/master/master.hpp           | 26 ++++++++++++++++----------
 src/master/readonly_handler.cpp | 19 +++++++++----------
 3 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/src/common/http.hpp b/src/common/http.hpp
index 76abafa..4f994a0 100644
--- a/src/common/http.hpp
+++ b/src/common/http.hpp
@@ -162,6 +162,7 @@ void json(JSON::ArrayWriter* writer, const Labels& labels);
 void json(JSON::ObjectWriter* writer, const MasterInfo& info);
 void json(
     JSON::StringWriter* writer, const MasterInfo::Capability& capability);
+void json(JSON::ObjectWriter* writer, const Offer& offer);
 void json(JSON::ObjectWriter* writer, const Resources& resources);
 void json(
     JSON::ObjectWriter* writer,
@@ -171,7 +172,6 @@ void json(
     JSON::StringWriter* writer, const SlaveInfo::Capability& capability);
 void json(JSON::ObjectWriter* writer, const Task& task);
 void json(JSON::ObjectWriter* writer, const TaskStatus& status);
-void json(JSON::ObjectWriter* writer, const Offer& offer);
 
 namespace authorization {
 
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 9b98771..eecb66c 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -1396,28 +1396,28 @@ private:
   public:
     explicit ReadOnlyHandler(const Master* _master) : master(_master) {}
 
-    // /state
-    process::http::Response state(
+    // /frameworks
+    process::http::Response frameworks(
         const process::http::Request& request,
         const process::Owned<ObjectApprovers>& approvers) const;
 
-    // /state-summary
-    process::http::Response stateSummary(
+    // /slaves
+    process::http::Response slaves(
         const process::http::Request& request,
         const process::Owned<ObjectApprovers>& approvers) const;
 
-    // /tasks
-    process::http::Response tasks(
+    // /state
+    process::http::Response state(
         const process::http::Request& request,
         const process::Owned<ObjectApprovers>& approvers) const;
 
-    // /slaves
-    process::http::Response slaves(
+    // /state-summary
+    process::http::Response stateSummary(
         const process::http::Request& request,
         const process::Owned<ObjectApprovers>& approvers) const;
 
-    // /frameworks
-    process::http::Response frameworks(
+    // /tasks
+    process::http::Response tasks(
         const process::http::Request& request,
         const process::Owned<ObjectApprovers>& approvers) const;
 
@@ -1466,6 +1466,8 @@ private:
             principal) const;
 
     // /master/frameworks
+    //
+    // NOTE: Requests to this endpoint are batched.
     process::Future<process::http::Response> frameworks(
         const process::http::Request& request,
         const Option<process::http::authentication::Principal>&
@@ -1498,6 +1500,8 @@ private:
             principal) const;
 
     // /master/slaves
+    //
+    // NOTE: Requests to this endpoint are batched.
     process::Future<process::http::Response> slaves(
         const process::http::Request& request,
         const Option<process::http::authentication::Principal>&
@@ -1520,6 +1524,8 @@ private:
             principal) const;
 
     // /master/tasks
+    //
+    // NOTE: Requests to this endpoint are batched.
     process::Future<process::http::Response> tasks(
         const process::http::Request& request,
         const Option<process::http::authentication::Principal>&
diff --git a/src/master/readonly_handler.cpp b/src/master/readonly_handler.cpp
index 8026c6d..47d7de5 100644
--- a/src/master/readonly_handler.cpp
+++ b/src/master/readonly_handler.cpp
@@ -16,15 +16,15 @@
 
 #include "master/master.hpp"
 
-#include <vector>
 #include <string>
+#include <vector>
 
 #include <mesos/mesos.hpp>
 
 #include <mesos/authorizer/authorizer.hpp>
 
-#include <process/owned.hpp>
 #include <process/http.hpp>
+#include <process/owned.hpp>
 
 #include <stout/foreach.hpp>
 #include <stout/hashmap.hpp>
@@ -36,7 +36,6 @@
 #include "common/build.hpp"
 #include "common/http.hpp"
 
-
 using process::Owned;
 
 using process::http::OK;
@@ -637,14 +636,16 @@ process::http::Response 
Master::ReadOnlyHandler::frameworks(
 {
   IDAcceptor<FrameworkID> selectFrameworkId(
       request.url.query.get("framework_id"));
+
   // This lambda is consumed before the outer lambda
   // returns, hence capture by reference is fine here.
-  auto frameworks = [this, &approvers, &selectFrameworkId](
+  const Master* master = this->master;
+  auto frameworks = [master, &approvers, &selectFrameworkId](
       JSON::ObjectWriter* writer) {
     // Model all of the frameworks.
     writer->field(
         "frameworks",
-        [this, &approvers, &selectFrameworkId](
+        [master, &approvers, &selectFrameworkId](
             JSON::ArrayWriter* writer) {
           foreachvalue (
               Framework* framework, master->frameworks.registered) {
@@ -662,7 +663,7 @@ process::http::Response Master::ReadOnlyHandler::frameworks(
     // Model all of the completed frameworks.
     writer->field(
         "completed_frameworks",
-        [this, &approvers, &selectFrameworkId](
+        [master, &approvers, &selectFrameworkId](
             JSON::ArrayWriter* writer) {
           foreachvalue (const Owned<Framework>& framework,
                         master->frameworks.completed) {
@@ -691,13 +692,11 @@ process::http::Response Master::ReadOnlyHandler::slaves(
     const process::http::Request& request,
     const process::Owned<ObjectApprovers>& approvers) const
 {
-  Option<string> slaveId = request.url.query.get("slave_id");
-  Option<string> jsonp = request.url.query.get("jsonp");
-  IDAcceptor<SlaveID> selectSlaveId(slaveId);
+  IDAcceptor<SlaveID> selectSlaveId(request.url.query.get("slave_id"));
 
   return process::http::OK(
       jsonify(SlavesWriter(master->slaves, approvers, selectSlaveId)),
-      jsonp);
+      request.url.query.get("jsonp"));
 }
 
 

Reply via email to