From 741773f63fa24d2c862db32cb46a0edfbd69968b Mon Sep 17 00:00:00 2001
From: Paul Cristian Sarbu <paul.cristian.sarbu@huawei.com>
Date: Tue, 16 Jul 2024 16:53:36 +0200
Subject: just: Move RetryConfig setup before starting just serve

As the serve endpoint acts also as a regular client to its
associated remote-execution endpoint, it should employ the same
retry strategy as the regular just client.

Also updates ReadJustServeConfig to store the retry config
arguments instead of calling the RetryConfig singleton setters
directly, thus clearly separating the reading of the arguments from
the creation of any configuration singleton/instance.
---
 src/buildtool/main/TARGETS   |  1 -
 src/buildtool/main/main.cpp  |  9 ++++++---
 src/buildtool/main/serve.cpp | 31 +++++++++----------------------
 3 files changed, 15 insertions(+), 26 deletions(-)

(limited to 'src')

diff --git a/src/buildtool/main/TARGETS b/src/buildtool/main/TARGETS
index ce7ec45f..c9294c48 100644
--- a/src/buildtool/main/TARGETS
+++ b/src/buildtool/main/TARGETS
@@ -286,7 +286,6 @@
     , ["src/buildtool/logging", "log_level"]
     , ["src/buildtool/logging", "logging"]
     , "common"
-    , ["src/buildtool/common/remote", "retry_config"]
     ]
   }
 , "build_utils":
diff --git a/src/buildtool/main/main.cpp b/src/buildtool/main/main.cpp
index 5e9ff7b8..1d6dafeb 100644
--- a/src/buildtool/main/main.cpp
+++ b/src/buildtool/main/main.cpp
@@ -855,6 +855,12 @@ auto main(int argc, char* argv[]) -> int {
             return kExitFailure;
         }
 
+        // Set up the retry arguments, needed only for the client-side logic of
+        // remote execution, i.e., just serve and the regular just client.
+        if (not SetupRetryConfig(arguments.retry)) {
+            return kExitFailure;
+        }
+
         if (arguments.cmd == SubCommand::kServe) {
             auto serve_server =
                 ServeServerImpl::Create(arguments.service.interface,
@@ -973,9 +979,6 @@ auto main(int argc, char* argv[]) -> int {
         Progress progress{};
 
 #ifndef BOOTSTRAP_BUILD_TOOL
-        if (not SetupRetryConfig(arguments.retry)) {
-            std::exit(kExitFailure);
-        }
         ApiBundle const main_apis{&*storage_config,
                                   &storage,
                                   &*local_exec_config,
diff --git a/src/buildtool/main/serve.cpp b/src/buildtool/main/serve.cpp
index e5dec606..87f1c9ac 100644
--- a/src/buildtool/main/serve.cpp
+++ b/src/buildtool/main/serve.cpp
@@ -31,7 +31,6 @@
 #include "src/buildtool/build_engine/expression/configuration.hpp"
 #include "src/buildtool/build_engine/expression/expression.hpp"
 #include "src/buildtool/common/location.hpp"
-#include "src/buildtool/common/remote/retry_config.hpp"
 #include "src/buildtool/file_system/file_system_manager.hpp"
 #include "src/buildtool/logging/log_level.hpp"
 #include "src/buildtool/logging/logger.hpp"
@@ -56,8 +55,9 @@ namespace {
 
 }  // namespace
 
-[[nodiscard]] auto ParseRetryCliOptions(Configuration const& config) noexcept
-    -> bool {
+[[nodiscard]] auto ParseRetryCliOptions(
+    Configuration const& config,
+    gsl::not_null<RetryArguments*> const& rargs) noexcept -> bool {
     auto max_attempts = config["max-attempts"];
     if (max_attempts.IsNotNull()) {
         if (!max_attempts->IsNumber()) {
@@ -67,12 +67,7 @@ namespace {
                 max_attempts->ToString());
             return false;
         }
-        if (!RetryConfig::SetMaxAttempts(max_attempts->Number())) {
-            Logger::Log(LogLevel::Error,
-                        "Invalid value for \"max-attempts\" {}.",
-                        max_attempts->Number());
-            return false;
-        }
+        rargs->max_attempts = static_cast<unsigned int>(max_attempts->Number());
     }
     auto initial_backoff = config["initial-backoff-seconds"];
     if (initial_backoff.IsNotNull()) {
@@ -83,12 +78,8 @@ namespace {
                         initial_backoff->ToString());
             return false;
         }
-        if (!RetryConfig::SetMaxAttempts(initial_backoff->Number())) {
-            Logger::Log(LogLevel::Error,
-                        "Invalid value for \"initial-backoff-seconds\" {}.",
-                        initial_backoff->Number());
-            return false;
-        }
+        rargs->initial_backoff_seconds =
+            static_cast<unsigned int>(initial_backoff->Number());
     }
     auto max_backoff = config["max-backoff-seconds"];
     if (max_backoff.IsNotNull()) {
@@ -99,12 +90,8 @@ namespace {
                         max_backoff->ToString());
             return false;
         }
-        if (!RetryConfig::SetMaxAttempts(max_backoff->Number())) {
-            Logger::Log(LogLevel::Error,
-                        "Invalid value for \"max-backoff-seconds\" {}.",
-                        max_backoff->Number());
-            return false;
-        }
+        rargs->max_backoff_seconds =
+            static_cast<unsigned int>(max_backoff->Number());
     }
     return true;
 }
@@ -420,7 +407,7 @@ void ReadJustServeConfig(gsl::not_null<CommandLineArguments*> const& clargs) {
             }
             clargs->endpoint.remote_execution_address = address->String();
         }
-        if (!ParseRetryCliOptions(serve_config)) {
+        if (!ParseRetryCliOptions(serve_config, &clargs->retry)) {
             std::exit(kExitFailure);
         }
     }
-- 
cgit v1.2.3