You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2012/09/10 00:02:54 UTC

svn commit: r1382590 - in /incubator/mesos/trunk/src: Makefile.am flags/flags.hpp flags/loader.hpp flags/parse.hpp

Author: benh
Date: Sun Sep  9 22:02:54 2012
New Revision: 1382590

URL: http://svn.apache.org/viewvc?rev=1382590&view=rev
Log:
Refactored flags loaders to take a "parse" function for better code
reuse and organization (https://reviews.apache.org/r/6824).

Added:
    incubator/mesos/trunk/src/flags/parse.hpp
Modified:
    incubator/mesos/trunk/src/Makefile.am
    incubator/mesos/trunk/src/flags/flags.hpp
    incubator/mesos/trunk/src/flags/loader.hpp

Modified: incubator/mesos/trunk/src/Makefile.am
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/Makefile.am?rev=1382590&r1=1382589&r2=1382590&view=diff
==============================================================================
--- incubator/mesos/trunk/src/Makefile.am (original)
+++ incubator/mesos/trunk/src/Makefile.am Sun Sep  9 22:02:54 2012
@@ -195,17 +195,16 @@ libmesos_no_third_party_la_SOURCES += co
 	configurator/configurator.hpp configurator/option.hpp		\
 	detector/detector.hpp examples/utils.hpp files/files.hpp	\
 	flags/flag.hpp flags/flags.hpp flags/loader.hpp			\
-	launcher/launcher.hpp linux/cgroups.hpp linux/fs.hpp		\
-	linux/proc.hpp local/flags.hpp local/local.hpp			\
+	flags/parse.hpp launcher/launcher.hpp linux/cgroups.hpp		\
+	linux/fs.hpp linux/proc.hpp local/flags.hpp local/local.hpp	\
 	logging/flags.hpp logging/logging.hpp master/allocator.hpp	\
-	master/constants.hpp master/drf_sorter.hpp			\
-	master/flags.hpp master/frameworks_manager.hpp			\
-	master/hierarchical_allocator_process.hpp			\
-	master/http.hpp master/master.hpp				\
-	master/slaves_manager.hpp master/sorter.hpp master/webui.hpp	\
-	messages/messages.hpp slave/constants.hpp slave/flags.hpp	\
-	slave/gc.hpp slave/http.hpp slave/isolation_module.hpp		\
-	slave/isolation_module_factory.hpp				\
+	master/constants.hpp master/drf_sorter.hpp master/flags.hpp	\
+	master/frameworks_manager.hpp					\
+	master/hierarchical_allocator_process.hpp master/http.hpp	\
+	master/master.hpp master/slaves_manager.hpp master/sorter.hpp	\
+	master/webui.hpp messages/messages.hpp slave/constants.hpp	\
+	slave/flags.hpp slave/gc.hpp slave/http.hpp			\
+	slave/isolation_module.hpp slave/isolation_module_factory.hpp	\
 	slave/cgroups_isolation_module.hpp				\
 	slave/lxc_isolation_module.hpp					\
 	slave/process_based_isolation_module.hpp slave/reaper.hpp	\

Modified: incubator/mesos/trunk/src/flags/flags.hpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/flags/flags.hpp?rev=1382590&r1=1382589&r2=1382590&view=diff
==============================================================================
--- incubator/mesos/trunk/src/flags/flags.hpp (original)
+++ incubator/mesos/trunk/src/flags/flags.hpp Sun Sep  9 22:02:54 2012
@@ -11,9 +11,11 @@
 
 #include <stout/option.hpp>
 #include <stout/stringify.hpp>
+#include <stout/try.hpp>
 
 #include "flags/flag.hpp"
 #include "flags/loader.hpp"
+#include "flags/parse.hpp"
 
 // An abstraction for application/library "flags". An example is
 // probably best:
@@ -95,14 +97,14 @@ public:
   const_iterator end() const { return flags.end(); }
 
 protected:
-  template <typename T, typename M1, typename M2>
-  void add(M1 T::*m1,
+  template <typename Flags, typename T1, typename T2>
+  void add(T1 Flags::*t1,
            const std::string& name,
            const std::string& help,
-           const M2& m2);
+           const T2& t2);
 
-  template <typename T, typename M>
-  void add(Option<M> T::*option,
+  template <typename Flags, typename T>
+  void add(Option<T> Flags::*option,
            const std::string& name,
            const std::string& help);
 
@@ -150,31 +152,33 @@ public:
 };
 
 
-template <typename T, typename M1, typename M2>
+template <typename Flags, typename T1, typename T2>
 void FlagsBase::add(
-    M1 T::*m1,
+    T1 Flags::*t1,
     const std::string& name,
     const std::string& help,
-    const M2& m2)
+    const T2& t2)
 {
-  T* t = dynamic_cast<T*>(this);
-  if (t == NULL) {
+  Flags* flags = dynamic_cast<Flags*>(this);
+  if (flags == NULL) {
     std::cerr << "Attempted to add flag '" << name
               << "' with incompatible type" << std::endl;
     abort();
   } else {
-    t->*m1 = m2; // Set the default.
+    flags->*t1 = t2; // Set the default.
   }
 
   Flag flag;
   flag.name = name;
   flag.help = help;
-  flag.boolean = typeid(M1) == typeid(bool);
+  flag.boolean = typeid(T1) == typeid(bool);
   flag.loader = std::tr1::bind(
-      &MemberLoader<T, M1>::load,
-      name,
-      m1,
+      &MemberLoader<Flags, T1>::load,
       std::tr1::placeholders::_1,
+      t1,
+      std::tr1::function<Try<T1>(const std::string&)>(
+          std::tr1::bind(&parse<T1>, std::tr1::placeholders::_1)),
+      name,
       std::tr1::placeholders::_2);
 
   // Update the help string to include the default value.
@@ -186,7 +190,7 @@ void FlagsBase::add(
   flag.help += help.size() > 0 && help.find_last_of("\n\r") != help.size() - 1
     ? " (default: " // On same line, add space.
     : "(default: "; // On newline.
-  flag.help += stringify(m2);
+  flag.help += stringify(t2);
   flag.help += ")";
 
   flag.help += ")";
@@ -195,14 +199,14 @@ void FlagsBase::add(
 }
 
 
-template <typename T, typename M>
+template <typename Flags, typename T>
 void FlagsBase::add(
-    Option<M> T::*option,
+    Option<T> Flags::*option,
     const std::string& name,
     const std::string& help)
 {
-  T* t = dynamic_cast<T*>(this);
-  if (t == NULL) {
+  Flags* flags = dynamic_cast<Flags*>(this);
+  if (flags == NULL) {
     std::cerr << "Attempted to add flag '" << name
               << "' with incompatible type" << std::endl;
     abort();
@@ -211,12 +215,14 @@ void FlagsBase::add(
   Flag flag;
   flag.name = name;
   flag.help = help;
-  flag.boolean = typeid(M) == typeid(bool);
+  flag.boolean = typeid(T) == typeid(bool);
   flag.loader = std::tr1::bind(
-      &OptionMemberLoader<T, M>::load,
-      name,
-      option,
+      &OptionMemberLoader<Flags, T>::load,
       std::tr1::placeholders::_1,
+      option,
+      std::tr1::function<Try<T>(const std::string&)>(
+          std::tr1::bind(&parse<T>, std::tr1::placeholders::_1)),
+      name,
       std::tr1::placeholders::_2);
 
   add(flag);
@@ -309,8 +315,10 @@ void Flags<Flags1, Flags2, Flags3, Flags
   flag.boolean = typeid(T1) == typeid(bool);
   flag.loader = std::tr1::bind(
       &Loader<T1>::load,
-      name,
       t1,
+      std::tr1::function<Try<T1>(const std::string&)>(
+          std::tr1::bind(&parse<T1>, std::tr1::placeholders::_1)),
+      name,
       std::tr1::placeholders::_2); // Use _2 because ignore FlagsBase*.
 
   // Update the help string to include the default value.
@@ -341,8 +349,10 @@ void Flags<Flags1, Flags2, Flags3, Flags
   flag.boolean = typeid(T) == typeid(bool);
   flag.loader = std::tr1::bind(
       &OptionLoader<T>::load,
-      name,
       option,
+      std::tr1::function<Try<T>(const std::string&)>(
+          std::tr1::bind(&parse<T>, std::tr1::placeholders::_1)),
+      name,
       std::tr1::placeholders::_2); // Use _2 because ignore FlagsBase*.
 
   FlagsBase::add(flag);

Modified: incubator/mesos/trunk/src/flags/loader.hpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/flags/loader.hpp?rev=1382590&r1=1382589&r2=1382590&view=diff
==============================================================================
--- incubator/mesos/trunk/src/flags/loader.hpp (original)
+++ incubator/mesos/trunk/src/flags/loader.hpp Sun Sep  9 22:02:54 2012
@@ -1,12 +1,12 @@
 #ifndef __FLAGS_LOADER_HPP__
 #define __FLAGS_LOADER_HPP__
 
-#include <sstream> // For istringstream.
 #include <string>
 
 #include <tr1/functional>
 
 #include <stout/option.hpp>
+#include <stout/try.hpp>
 
 namespace flags {
 
@@ -16,49 +16,18 @@ class FlagsBase;
 template <typename T>
 struct Loader
 {
-  static void load(const std::string& name,
-                   T* t,
+  static void load(T* flag,
+                   const std::tr1::function<Try<T>(const std::string&)>& parse,
+                   const std::string& name,
                    const std::string& value)
   {
-    std::istringstream in(value);
-    in >> *t;
-    if (!in.good() && !in.eof()) {
-      std::cerr << "Failed to load value '" << value
-                << "' for flag '" << name
-                << "'" << std::endl;
-      abort();
-    }
-  }
-};
-
-
-template <>
-struct Loader<std::string>
-{
-  static void load(const std::string& name,
-                   std::string* s,
-                   const std::string& value)
-  {
-    *s = value;
-  }
-};
-
-
-template <>
-struct Loader<bool>
-{
-  static void load(const std::string& name,
-                   bool* b,
-                   const std::string& value)
-  {
-    if (value == "true" || value == "1") {
-      *b = true;
-    } else if (value == "false" || value == "0") {
-      *b = false;
+    Try<T> t = parse(value);
+    if (t.isSome()) {
+      *flag = t.get();
     } else {
       std::cerr << "Failed to load value '" << value
                 << "' for flag '" << name
-                << "'" << std::endl;
+                << "': " << t.error() << std::endl;
       abort();
     }
   }
@@ -68,114 +37,42 @@ struct Loader<bool>
 template <typename T>
 struct OptionLoader
 {
-  static void load(const std::string& name,
-                   Option<T>* option,
-                   const std::string& value)
-  {
-    T t;
-    std::istringstream in(value);
-    in >> t;
-    if (!in.good() && !in.eof()) {
-      std::cerr << "Failed to load value '" << value
-                << "' for flag '" << name
-                << "'" << std::endl;
-      abort();
-    }
-    *option = Option<T>::some(t);
-  }
-};
-
-
-template <>
-struct OptionLoader<std::string>
-{
-  static void load(const std::string& name,
-                   Option<std::string>* option,
-                   const std::string& value)
-  {
-    *option = Option<std::string>(value);
-  }
-};
-
-
-template <>
-struct OptionLoader<bool>
-{
-  static void load(const std::string& name,
-                   Option<bool>* option,
+  static void load(Option<T>* flag,
+                   const std::tr1::function<Try<T>(const std::string&)>& parse,
+                   const std::string& name,
                    const std::string& value)
   {
-    if (value == "true" || value == "1") {
-      *option = Option<bool>::some(true);
-    } else if (value == "false" || value == "0") {
-      *option = Option<bool>::some(false);
+    Try<T> t = parse(value);
+    if (t.isSome()) {
+      *flag = Option<T>::some(t.get());
     } else {
       std::cerr << "Failed to load value '" << value
                 << "' for flag '" << name
-                << "'" << std::endl;
+                << "': " << t.error() << std::endl;
       abort();
     }
   }
 };
 
 
-template <typename T, typename M>
+template <typename F, typename T>
 struct MemberLoader
 {
-  static void load(const std::string& name,
-                   M T::*m,
-                   FlagsBase* base,
-                   const std::string& value)
-  {
-    T* t = dynamic_cast<T*>(base);
-    if (t != NULL) {
-      std::istringstream in(value);
-      in >> t->*m;
-      if (!in.good() && !in.eof()) {
-        std::cerr << "Failed to load value '" << value
-                  << "' for flag '" << name
-                  << "'" << std::endl;
-        abort();
-      }
-    }
-  }
-};
-
-
-template <typename T>
-struct MemberLoader<T, std::string>
-{
-  static void load(const std::string& name,
-                   std::string T::*s,
-                   FlagsBase* base,
-                   const std::string& value)
-  {
-    T* t = dynamic_cast<T*>(base);
-    if (t != NULL) {
-      t->*s = value;
-    }
-  }
-};
-
-
-template <typename T>
-struct MemberLoader<T, bool>
-{
-  static void load(const std::string& name,
-                   bool T::*b,
-                   FlagsBase* base,
+  static void load(FlagsBase* base,
+                   T F::*flag,
+                   const std::tr1::function<Try<T>(const std::string&)>& parse,
+                   const std::string& name,
                    const std::string& value)
   {
-    T* t = dynamic_cast<T*>(base);
-    if (t != NULL) {
-      if (value == "true" || value == "1") {
-        t->*b = true;
-      } else if (value == "false" || value == "0") {
-        t->*b = false;
+    F* f = dynamic_cast<F*>(base);
+    if (f != NULL) {
+      Try<T> t = parse(value);
+      if (t.isSome()) {
+        f->*flag = t.get();
       } else {
         std::cerr << "Failed to load value '" << value
                   << "' for flag '" << name
-                  << "'" << std::endl;
+                  << "': " << t.error() << std::endl;
         abort();
       }
     }
@@ -183,65 +80,24 @@ struct MemberLoader<T, bool>
 };
 
 
-template <typename T, typename M>
+template <typename F, typename T>
 struct OptionMemberLoader
 {
-  static void load(const std::string& name,
-                   Option<M> T::*option,
-                   FlagsBase* base,
-                   const std::string& value)
-  {
-    T* t = dynamic_cast<T*>(base);
-    if (t != NULL) {
-      M m;
-      std::istringstream in(value);
-      in >> m;
-      if (!in.good() && !in.eof()) {
-        std::cerr << "Failed to load value '" << value
-                  << "' for flag '" << name
-                  << "'" << std::endl;
-        abort();
-      }
-      t->*option = Option<T>::some(m);
-    }
-  }
-};
-
-
-template <typename T>
-struct OptionMemberLoader<T, std::string>
-{
-  static void load(const std::string& name,
-                   Option<std::string> T::*option,
-                   FlagsBase* base,
-                   const std::string& value)
-  {
-    T* t = dynamic_cast<T*>(base);
-    if (t != NULL) {
-      t->*option = Option<std::string>(value);
-    }
-  }
-};
-
-
-template <typename T>
-struct OptionMemberLoader<T, bool>
-{
-  static void load(const std::string& name,
-                   Option<bool> T::*option,
-                   FlagsBase* base,
+  static void load(FlagsBase* base,
+                   Option<T> F::*flag,
+                   const std::tr1::function<Try<T>(const std::string&)>& parse,
+                   const std::string& name,
                    const std::string& value)
   {
-    T* t = dynamic_cast<T*>(base);
-    if (t != NULL) {
-      if (value == "true" || value == "1") {
-        t->*option = Option<bool>::some(true);
-      } else if (value == "false" || value == "0") {
-        t->*option = Option<bool>::some(false);
+    F* f = dynamic_cast<F*>(base);
+    if (f != NULL) {
+      Try<T> t = parse(value);
+      if (t.isSome()) {
+        f->*flag = Option<T>::some(t.get());
       } else {
         std::cerr << "Failed to load value '" << value
                   << "' for flag '" << name
-                  << "'" << std::endl;
+                  << "': " << t.error() << std::endl;
         abort();
       }
     }

Added: incubator/mesos/trunk/src/flags/parse.hpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/flags/parse.hpp?rev=1382590&view=auto
==============================================================================
--- incubator/mesos/trunk/src/flags/parse.hpp (added)
+++ incubator/mesos/trunk/src/flags/parse.hpp Sun Sep  9 22:02:54 2012
@@ -0,0 +1,46 @@
+#ifndef __FLAGS_PARSE_HPP__
+#define __FLAGS_PARSE_HPP__
+
+#include <sstream> // For istringstream.
+#include <string>
+
+#include <tr1/functional>
+
+#include <stout/try.hpp>
+
+namespace flags {
+
+template <typename T>
+Try<T> parse(const std::string& value)
+{
+  T t;
+  std::istringstream in(value);
+  in >> t;
+  if (!in.good() && !in.eof()) {
+    return Try<T>::error("Could not parse into required type");
+  }
+  return t;
+}
+
+
+template <>
+inline Try<std::string> parse(const std::string& value)
+{
+  return value;
+}
+
+
+template <>
+inline Try<bool> parse(const std::string& value)
+{
+  if (value == "true" || value == "1") {
+    return true;
+  } else if (value == "false" || value == "0") {
+    return false;
+  }
+  return Try<bool>::error("Expecting a boolean (e.g., true or false)");
+}
+
+} // namespace flags {
+
+#endif // __FLAGS_PARSE_HPP__