[mesos] 01/10: Removed unnecessary failure handling in agent HTTP API handlers.

[ch...@apache.org]

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

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

commit 0581437999a262277f695592c6a1be9d30bf8c31
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
AuthorDate: Tue Sep 18 12:59:52 2018 -0700

    Removed unnecessary failure handling in agent HTTP API handlers.
    
    The current agent HTTP API handlers either unnecessarily handle failures
    on always-ready futures, or return "500 Internal Server Error"
    unnecessarily. This patch removes those unnecessarily code.
    
    Review: https://reviews.apache.org/r/68755
---
 src/slave/http.cpp | 91 ++++++------------------------------------------------
 1 file changed, 9 insertions(+), 82 deletions(-)

diff --git a/src/slave/http.cpp b/src/slave/http.cpp
index 0011ced..0adc7c0 100644
--- a/src/slave/http.cpp
+++ b/src/slave/http.cpp
@@ -2085,21 +2085,11 @@ Future<Response> Http::getContainers(
               call.get_containers().show_nested(),
               call.get_containers().show_standalone());
         }))
-    .then([acceptType](const Future<JSON::Array>& result) -> Response {
-      if (!result.isReady()) {
-        LOG(WARNING) << "Could not collect container status and statistics: "
-                     << (result.isFailed()
-                          ? result.failure()
-                          : "Discarded");
-        return result.isFailed()
-          ? InternalServerError(result.failure())
-          : InternalServerError();
-      }
-
+    .then([acceptType](const JSON::Array& result) -> Response {
       return OK(
           serialize(
               acceptType,
-              evolve<v1::agent::Response::GET_CONTAINERS>(result.get())),
+              evolve<v1::agent::Response::GET_CONTAINERS>(result)),
           stringify(acceptType));
     });
 }
@@ -2126,20 +2116,8 @@ Future<Response> Http::_containers(
               false,
               false);
         }))
-    .then([request](const Future<JSON::Array>& result) -> Response {
-      if (!result.isReady()) {
-        LOG(WARNING) << "Could not collect container status and statistics: "
-                     << (result.isFailed()
-                         ? result.failure()
-                         : "Discarded");
-
-        return result.isFailed()
-            ? InternalServerError(result.failure())
-            : InternalServerError();
-      }
-
-      return process::http::OK(
-          result.get(), request.url.query.get("jsonp"));
+    .then([request](const JSON::Array& result) -> Response {
+      return process::http::OK(result, request.url.query.get("jsonp"));
     });
 }
 
@@ -2342,23 +2320,7 @@ Future<Response> Http::pruneImages(
           }
 
           return slave->containerizer->pruneImages(excludedImages)
-            .then([](const Future<Nothing>& result) -> Response {
-                if (!result.isReady()) {
-                  // TODO(zhitao): Because `containerizer::pruneImages` returns
-                  // a `Nothing` now, we cannot distinguish between actual
-                  // failure or the case that operator should drain the agent.
-                  // Consider returning more information.
-                  LOG(WARNING)
-                      << "Failed to prune images: "
-                      << (result.isFailed() ? result.failure() : "discarded");
-
-                  return result.isFailed()
-                      ? InternalServerError(result.failure())
-                      : InternalServerError();
-                }
-
-                return OK();
-              });
+            .then([]() -> Response { return OK(); });
         }));
 }
 
@@ -3056,15 +3018,7 @@ Future<Response> Http::_removeContainer(
   Future<Nothing> remove = slave->containerizer->remove(containerId);
 
   return remove
-    .then([=](const Future<Nothing>& result) -> Response {
-      if (result.isFailed()) {
-        LOG(ERROR) << "Failed to remove container " << containerId
-                   << ": " << result.failure();
-        return InternalServerError(result.failure());
-      }
-
-      return OK();
-    });
+    .then([=]() -> Response { return OK(); });
 }
 
 
@@ -3259,15 +3213,7 @@ Future<Response> Http::addResourceProviderConfig(
               }
 
               return OK();
-            })
-            .repair([info](const Future<Response>& future) {
-                LOG(ERROR)
-                    << "Failed to add resource provider config with type '"
-                    << info.type() << "' and name '" << info.name() << "': "
-                    << future.failure();
-
-                return InternalServerError(future.failure());
-              });
+            });
         }));
 }
 
@@ -3304,15 +3250,7 @@ Future<Response> Http::updateResourceProviderConfig(
               }
 
               return OK();
-            })
-            .repair([info](const Future<Response>& future) {
-                LOG(ERROR)
-                    << "Failed to update resource provider config with type '"
-                    << info.type() << "' and name '" << info.name() << "': "
-                    << future.failure();
-
-                return InternalServerError(future.failure());
-              });
+            });
         }));
 }
 
@@ -3343,18 +3281,7 @@ Future<Response> Http::removeResourceProviderConfig(
               << type << "' and name '" << name << "'";
 
           return slave->localResourceProviderDaemon->remove(type, name)
-            .then([]() -> Response {
-              // Config removal is always successful due to idempotency.
-              return OK();
-            })
-            .repair([type, name](const Future<Response>& future) {
-              LOG(ERROR)
-                  << "Failed to remove resource provider config with type '"
-                  << type << "' and name '" << name << "': "
-                  << future.failure();
-
-              return InternalServerError(future.failure());
-            });
+            .then([]() -> Response { return OK(); });
       }));
 }