Repository: mesos
Updated Branches:
  refs/heads/master 2abd9c8ae -> ffddcfb53


Remove string::format and unnecessary constants from slave paths.

Review: https://reviews.apache.org/r/30531


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/7075ef8e
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/7075ef8e
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/7075ef8e

Branch: refs/heads/master
Commit: 7075ef8e5e6ef1a8f1962598398aa6cf79b711b9
Parents: 2abd9c8
Author: Dominic Hamon <[email protected]>
Authored: Wed Feb 4 09:16:21 2015 -0800
Committer: Dominic Hamon <[email protected]>
Committed: Wed Feb 4 10:00:00 2015 -0800

----------------------------------------------------------------------
 src/slave/paths.cpp | 306 ++++++++++++++++++-----------------------------
 src/slave/paths.hpp |   2 +-
 2 files changed, 120 insertions(+), 188 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/7075ef8e/src/slave/paths.cpp
----------------------------------------------------------------------
diff --git a/src/slave/paths.cpp b/src/slave/paths.cpp
index fe951ab..5fcfa2c 100644
--- a/src/slave/paths.cpp
+++ b/src/slave/paths.cpp
@@ -22,10 +22,10 @@
 #include <mesos/mesos.hpp>
 
 #include <stout/check.hpp>
-#include <stout/format.hpp>
 #include <stout/fs.hpp>
 #include <stout/nothing.hpp>
 #include <stout/os.hpp>
+#include <stout/path.hpp>
 #include <stout/try.hpp>
 
 #include "common/type_utils.hpp"
@@ -42,63 +42,18 @@ namespace internal {
 namespace slave {
 namespace paths {
 
-// TODO(bmahler): Replace these non-POD globals, per MESOS-1023.
-
 // File names.
-const string BOOT_ID_FILE = "boot_id";
-const string SLAVE_INFO_FILE = "slave.info";
-const string FRAMEWORK_PID_FILE = "framework.pid";
-const string FRAMEWORK_INFO_FILE = "framework.info";
-const string LIBPROCESS_PID_FILE = "libprocess.pid";
-const string EXECUTOR_INFO_FILE = "executor.info";
-const string EXECUTOR_SENTINEL_FILE = "executor.sentinel";
-const string FORKED_PID_FILE = "forked.pid";
-const string TASK_INFO_FILE = "task.info";
-const string TASK_UPDATES_FILE = "task.updates";
-const string RESOURCES_INFO_FILE = "resources.info";
-
-// Path layout templates.
-const string ROOT_PATH = "%s";
-const string LATEST_SLAVE_PATH =
-  path::join(ROOT_PATH, "slaves", LATEST_SYMLINK);
-const string SLAVE_PATH =
-  path::join(ROOT_PATH, "slaves", "%s");
-const string BOOT_ID_PATH =
-  path::join(ROOT_PATH, BOOT_ID_FILE);
-const string SLAVE_INFO_PATH =
-  path::join(SLAVE_PATH, SLAVE_INFO_FILE);
-const string FRAMEWORK_PATH =
-  path::join(SLAVE_PATH, "frameworks", "%s");
-const string FRAMEWORK_PID_PATH =
-  path::join(FRAMEWORK_PATH, FRAMEWORK_PID_FILE);
-const string FRAMEWORK_INFO_PATH =
-  path::join(FRAMEWORK_PATH, FRAMEWORK_INFO_FILE);
-const string EXECUTOR_PATH =
-  path::join(FRAMEWORK_PATH, "executors", "%s");
-const string EXECUTOR_INFO_PATH =
-  path::join(EXECUTOR_PATH, EXECUTOR_INFO_FILE);
-const string EXECUTOR_RUN_PATH =
-  path::join(EXECUTOR_PATH, "runs", "%s");
-const string EXECUTOR_SENTINEL_PATH =
-  path::join(EXECUTOR_RUN_PATH, EXECUTOR_SENTINEL_FILE);
-const string EXECUTOR_LATEST_RUN_PATH =
-  path::join(EXECUTOR_PATH, "runs", LATEST_SYMLINK);
-const string PIDS_PATH =
-  path::join(EXECUTOR_RUN_PATH, "pids");
-const string LIBPROCESS_PID_PATH =
-  path::join(PIDS_PATH, LIBPROCESS_PID_FILE);
-const string FORKED_PID_PATH =
-  path::join(PIDS_PATH, FORKED_PID_FILE);
-const string TASK_PATH =
-  path::join(EXECUTOR_RUN_PATH, "tasks", "%s");
-const string TASK_INFO_PATH =
-  path::join(TASK_PATH, TASK_INFO_FILE);
-const string TASK_UPDATES_PATH =
-  path::join(TASK_PATH, TASK_UPDATES_FILE);
-const string RESOURCES_INFO_PATH =
-  path::join(ROOT_PATH, "resources", RESOURCES_INFO_FILE);
-const string PERSISTENT_VOLUME_PATH =
-  path::join(ROOT_PATH, "volumes", "roles", "%s", "%s");
+const char BOOT_ID_FILE[] = "boot_id";
+const char SLAVE_INFO_FILE[] = "slave.info";
+const char FRAMEWORK_PID_FILE[] = "framework.pid";
+const char FRAMEWORK_INFO_FILE[] = "framework.info";
+const char LIBPROCESS_PID_FILE[] = "libprocess.pid";
+const char EXECUTOR_INFO_FILE[] = "executor.info";
+const char EXECUTOR_SENTINEL_FILE[] = "executor.sentinel";
+const char FORKED_PID_FILE[] = "forked.pid";
+const char TASK_INFO_FILE[] = "task.info";
+const char TASK_UPDATES_FILE[] = "task.updates";
+const char RESOURCES_INFO_FILE[] = "resources.info";
 
 
 string getMetaRootDir(const string& rootDir)
@@ -113,31 +68,31 @@ string getArchiveDir(const string& rootDir)
 }
 
 
-string getLatestSlavePath(const string& rootDir)
+string getBootIdPath(const string& rootDir)
 {
-  return strings::format(LATEST_SLAVE_PATH, rootDir).get();
+  return path::join(rootDir, BOOT_ID_FILE);
 }
 
 
-string getBootIdPath(const string& rootDir)
+string getLatestSlavePath(const string& rootDir)
 {
-  return strings::format(BOOT_ID_PATH, rootDir).get();
+  return path::join(rootDir, "slaves", LATEST_SYMLINK);
 }
 
 
-string getSlaveInfoPath(
+string getSlavePath(
     const string& rootDir,
     const SlaveID& slaveId)
 {
-  return strings::format(SLAVE_INFO_PATH, rootDir, slaveId).get();
+  return path::join(rootDir, "slaves", stringify(slaveId));
 }
 
 
-string getSlavePath(
+string getSlaveInfoPath(
     const string& rootDir,
     const SlaveID& slaveId)
 {
-  return strings::format(SLAVE_PATH, rootDir, slaveId).get();
+  return path::join(getSlavePath(rootDir, slaveId), SLAVE_INFO_FILE);
 }
 
 
@@ -145,15 +100,8 @@ Try<list<string>> getFrameworkPaths(
     const string& rootDir,
     const SlaveID& slaveId)
 {
-  Try<string> format = strings::format(
-      paths::FRAMEWORK_PATH,
-      rootDir,
-      slaveId,
-      "*");
-
-  CHECK_SOME(format);
-
-  return os::glob(format.get());
+  return os::glob(
+      path::join(getSlavePath(rootDir, slaveId), "frameworks", "*"));
 }
 
 
@@ -162,8 +110,8 @@ string getFrameworkPath(
     const SlaveID& slaveId,
     const FrameworkID& frameworkId)
 {
-  return strings::format(
-      FRAMEWORK_PATH, rootDir, slaveId, frameworkId).get();
+  return path::join(
+      getSlavePath(rootDir, slaveId), "frameworks", stringify(frameworkId));
 }
 
 
@@ -172,8 +120,8 @@ string getFrameworkPidPath(
     const SlaveID& slaveId,
     const FrameworkID& frameworkId)
 {
-  return strings::format(
-      FRAMEWORK_PID_PATH, rootDir, slaveId, frameworkId).get();
+  return path::join(
+      getFrameworkPath(rootDir, slaveId, frameworkId), FRAMEWORK_PID_FILE);
 }
 
 
@@ -182,8 +130,8 @@ string getFrameworkInfoPath(
     const SlaveID& slaveId,
     const FrameworkID& frameworkId)
 {
-  return strings::format(
-      FRAMEWORK_INFO_PATH, rootDir, slaveId, frameworkId).get();
+  return path::join(
+      getFrameworkPath(rootDir, slaveId, frameworkId), FRAMEWORK_INFO_FILE);
 }
 
 
@@ -192,16 +140,10 @@ Try<list<string>> getExecutorPaths(
     const SlaveID& slaveId,
     const FrameworkID& frameworkId)
 {
-  Try<string> format = strings::format(
-      paths::EXECUTOR_PATH,
-      rootDir,
-      slaveId,
-      frameworkId,
-      "*");
-
-  CHECK_SOME(format);
-
-  return os::glob(format.get());
+  return os::glob(path::join(
+      getFrameworkPath(rootDir, slaveId, frameworkId),
+      "executors",
+      "*"));
 }
 
 
@@ -211,8 +153,10 @@ string getExecutorPath(
     const FrameworkID& frameworkId,
     const ExecutorID& executorId)
 {
-  return strings::format(
-      EXECUTOR_PATH, rootDir, slaveId, frameworkId, executorId).get();
+  return path::join(
+        getFrameworkPath(rootDir, slaveId, frameworkId),
+        "executors",
+        stringify(executorId));
 }
 
 
@@ -222,8 +166,9 @@ string getExecutorInfoPath(
     const FrameworkID& frameworkId,
     const ExecutorID& executorId)
 {
-  return strings::format(
-      EXECUTOR_INFO_PATH, rootDir, slaveId, frameworkId, executorId).get();
+  return path::join(
+      getExecutorPath(rootDir, slaveId, frameworkId, executorId),
+      EXECUTOR_INFO_FILE);
 }
 
 
@@ -233,17 +178,10 @@ Try<list<string>> getExecutorRunPaths(
     const FrameworkID& frameworkId,
     const ExecutorID& executorId)
 {
-  Try<string> format = strings::format(
-      paths::EXECUTOR_RUN_PATH,
-      rootDir,
-      slaveId,
-      frameworkId,
-      executorId,
-      "*");
-
-  CHECK_SOME(format);
-
-  return os::glob(format.get());
+  return os::glob(path::join(
+      getExecutorPath(rootDir, slaveId, frameworkId, executorId),
+      "runs",
+      "*"));
 }
 
 
@@ -254,13 +192,10 @@ string getExecutorRunPath(
     const ExecutorID& executorId,
     const ContainerID& containerId)
 {
-  return strings::format(
-      EXECUTOR_RUN_PATH,
-      rootDir,
-      slaveId,
-      frameworkId,
-      executorId,
-      containerId).get();
+  return path::join(
+      getExecutorPath(rootDir, slaveId, frameworkId, executorId),
+      "runs",
+      stringify(containerId));
 }
 
 
@@ -271,13 +206,14 @@ string getExecutorSentinelPath(
     const ExecutorID& executorId,
     const ContainerID& containerId)
 {
-  return strings::format(
-      EXECUTOR_SENTINEL_PATH,
-      rootDir,
-      slaveId,
-      frameworkId,
-      executorId,
-      containerId).get();
+  return path::join(
+      getExecutorRunPath(
+          rootDir,
+          slaveId,
+          frameworkId,
+          executorId,
+          containerId),
+      EXECUTOR_SENTINEL_FILE);
 }
 
 
@@ -287,12 +223,10 @@ string getExecutorLatestRunPath(
     const FrameworkID& frameworkId,
     const ExecutorID& executorId)
 {
-  return strings::format(
-      EXECUTOR_LATEST_RUN_PATH,
-      rootDir,
-      slaveId,
-      frameworkId,
-      executorId).get();
+  return path::join(
+      getExecutorPath(rootDir, slaveId, frameworkId, executorId),
+      "runs",
+      LATEST_SYMLINK);
 }
 
 
@@ -303,13 +237,15 @@ string getLibprocessPidPath(
     const ExecutorID& executorId,
     const ContainerID& containerId)
 {
-  return strings::format(
-      LIBPROCESS_PID_PATH,
-      rootDir,
-      slaveId,
-      frameworkId,
-      executorId,
-      containerId).get();
+  return path::join(
+      getExecutorRunPath(
+          rootDir,
+          slaveId,
+          frameworkId,
+          executorId,
+          containerId),
+      "pids",
+      LIBPROCESS_PID_FILE);
 }
 
 
@@ -320,13 +256,15 @@ string getForkedPidPath(
     const ExecutorID& executorId,
     const ContainerID& containerId)
 {
-  return strings::format(
-      FORKED_PID_PATH,
-      rootDir,
-      slaveId,
-      frameworkId,
-      executorId,
-      containerId).get();
+  return path::join(
+      getExecutorRunPath(
+          rootDir,
+          slaveId,
+          frameworkId,
+          executorId,
+          containerId),
+      "pids",
+      FORKED_PID_FILE);
 }
 
 
@@ -337,18 +275,15 @@ Try<list<string>> getTaskPaths(
     const ExecutorID& executorId,
     const ContainerID& containerId)
 {
-  Try<string> format = strings::format(
-      paths::TASK_PATH,
-      rootDir,
-      slaveId,
-      frameworkId,
-      executorId,
-      containerId,
-      "*");
-
-  CHECK_SOME(format);
-
-  return os::glob(format.get());
+  return os::glob(path::join(
+      getExecutorRunPath(
+          rootDir,
+          slaveId,
+          frameworkId,
+          executorId,
+          containerId),
+      "tasks",
+      "*"));
 }
 
 
@@ -360,14 +295,15 @@ string getTaskPath(
     const ContainerID& containerId,
     const TaskID& taskId)
 {
-  return strings::format(
-      TASK_PATH,
-      rootDir,
-      slaveId,
-      frameworkId,
-      executorId,
-      containerId,
-      taskId).get();
+  return path::join(
+      getExecutorRunPath(
+          rootDir,
+          slaveId,
+          frameworkId,
+          executorId,
+          containerId),
+      "tasks",
+      stringify(taskId));
 }
 
 
@@ -379,14 +315,15 @@ string getTaskInfoPath(
     const ContainerID& containerId,
     const TaskID& taskId)
 {
-  return strings::format(
-      TASK_INFO_PATH,
-      rootDir,
-      slaveId,
-      frameworkId,
-      executorId,
-      containerId,
-      taskId).get();
+  return path::join(
+      getTaskPath(
+          rootDir,
+          slaveId,
+          frameworkId,
+          executorId,
+          containerId,
+          taskId),
+      TASK_INFO_FILE);
 }
 
 
@@ -398,23 +335,22 @@ string getTaskUpdatesPath(
     const ContainerID& containerId,
     const TaskID& taskId)
 {
-  return strings::format(
-      TASK_UPDATES_PATH,
-      rootDir,
-      slaveId,
-      frameworkId,
-      executorId,
-      containerId,
-      taskId).get();
+  return path::join(
+      getTaskPath(
+          rootDir,
+          slaveId,
+          frameworkId,
+          executorId,
+          containerId,
+          taskId),
+      TASK_UPDATES_FILE);
 }
 
 
 string getResourcesInfoPath(
     const string& rootDir)
 {
-  return strings::format(
-      RESOURCES_INFO_PATH,
-      rootDir).get();
+  return path::join(rootDir, "resources", RESOURCES_INFO_FILE);
 }
 
 
@@ -423,11 +359,7 @@ string getPersistentVolumePath(
     const string& role,
     const string& persistenceId)
 {
-  return strings::format(
-      PERSISTENT_VOLUME_PATH,
-      rootDir,
-      role,
-      persistenceId).get();
+  return path::join(rootDir, "volumes", "roles", role, persistenceId);
 }
 
 
@@ -438,7 +370,7 @@ string createExecutorDirectory(
     const ExecutorID& executorId,
     const ContainerID& containerId)
 {
-  string directory =
+  const string directory =
     getExecutorRunPath(rootDir, slaveId, frameworkId, executorId, containerId);
 
   Try<Nothing> mkdir = os::mkdir(directory);
@@ -447,7 +379,7 @@ string createExecutorDirectory(
     << "Failed to create executor directory '" << directory << "'";
 
   // Remove the previous "latest" symlink.
-  string latest =
+  const string latest =
     getExecutorLatestRunPath(rootDir, slaveId, frameworkId, executorId);
 
   if (os::exists(latest)) {
@@ -470,7 +402,7 @@ string createSlaveDirectory(
     const string& rootDir,
     const SlaveID& slaveId)
 {
-  string directory = getSlavePath(rootDir, slaveId);
+  const string directory = getSlavePath(rootDir, slaveId);
 
   Try<Nothing> mkdir = os::mkdir(directory);
 
@@ -478,7 +410,7 @@ string createSlaveDirectory(
     << "Failed to create slave directory '" << directory << "'";
 
   // Remove the previous "latest" symlink.
-  string latest = getLatestSlavePath(rootDir);
+  const string latest = getLatestSlavePath(rootDir);
 
   if (os::exists(latest)) {
     CHECK_SOME(os::rm(latest))

http://git-wip-us.apache.org/repos/asf/mesos/blob/7075ef8e/src/slave/paths.hpp
----------------------------------------------------------------------
diff --git a/src/slave/paths.hpp b/src/slave/paths.hpp
index 4d6897f..1618439 100644
--- a/src/slave/paths.hpp
+++ b/src/slave/paths.hpp
@@ -89,7 +89,7 @@ namespace paths {
 //           |-- <role>
 //               |-- <persistence_id> (persistent volume)
 
-const std::string LATEST_SYMLINK = "latest";
+const char LATEST_SYMLINK[] = "latest";
 
 // Helpers for obtaining paths in the layout:
 

Reply via email to