Skip to content

Replace floating-point seconds with integer milliseconds in timing config #388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shegazyy wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Replace floating-point seconds with integer milliseconds in timing config #388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/demo_verification/lifecycle_demo_test.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"schema_version": 1,
"schema_version": 2,
"defaults": {
"deployment_config": {
"bin_dir": "/tmp/tests/examples",
Expand Down
2 changes: 1 addition & 1 deletion score/launch_manager/src/daemon/src/configuration/config_schema/examples/example_conf.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"schema_version": 1,
"schema_version": 2,
"defaults": {
"deployment_config": {
"ready_timeout": 0.5,
Expand Down
2 changes: 1 addition & 1 deletion score/launch_manager/src/daemon/src/configuration/config_schema/launch_manager.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@
"type": "integer",
"description": "Specifies the schema version number that the Launch Manager uses to determine how to parse and validate this configuration file.",
"enum": [
1
2

@NicolasFussberger NicolasFussberger Jul 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have the difficulty a bit that we are currently in transition.
The new_lm_flatcfg.fbs is the new flatbuffer schema that shall replace the legacy flatbuffer configs.
Though this is currently still only under a feature flag active. The regular build is still using the legacy configs.

My proposal for this PR would be:

  • We keep the adaptation for the new config path
  • We do not adapt the old config anymore as this is removed soon
  • We leave the user-facing json schema unchanged for now, and adapt this once the feature flag is gone and the new config is in use. Then we'll change the user-facing json config from seconds to ms as well.
  • This means the translation btw. seconds->ms is done in the lifecycle_config.py for now, as the user-facing json is still configured with seconds (unchanged) and the new flatbuffer schema expects milliseconds (changed).

I think this matches what you already did, only the user-facing json schema change would need to be reverted. As the user-facing json schema is anyway not changed here, I think we don't need to increase the schema version.

What do you think?

]
},
"defaults": {
Expand Down
182 changes: 109 additions & 73 deletions score/launch_manager/src/daemon/src/configuration/details/flatbuffer_config_loader_UT.cpp
View file
Open in desktop

Large diffs are not rendered by default.

93 changes: 11 additions & 82 deletions score/launch_manager/src/daemon/src/configuration/details/flatbuffer_type_converters.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include <score/assert.hpp>
#include <sys/types.h>
#include <cstdint>
#include <limits>
#include <string>
#include <vector>

Expand All @@ -34,8 +33,9 @@ namespace
{

template <typename T>
score::cpp::expected<T, IConfigLoader::Error> requireScalarValue(const ::flatbuffers::Optional<T>& field,
const std::string_view field_name)
score::cpp::expected<T, IConfigLoader::Error> requireScalarValue(
const ::flatbuffers::Optional<T>& field,
const std::string_view field_name)
{
if (!field.has_value())
{
Expand All @@ -56,29 +56,6 @@ std::optional<T> optionalScalarValue(const ::flatbuffers::Optional<T>& field)
namespace details
{

constexpr double kSecondsToMilliseconds = 1000.0;

score::cpp::expected<uint32_t, IConfigLoader::Error> secondsToMs(double seconds)
{
if (seconds < 0.0)
{
LM_LOG_ERROR() << "Negative time value " << seconds << " seconds is not supported";
return score::cpp::make_unexpected(IConfigLoader::Error::InvalidFormat);
}
if (seconds * kSecondsToMilliseconds > static_cast<double>(std::numeric_limits<uint32_t>::max()))
{
LM_LOG_ERROR() << "Time value " << seconds << " seconds exceeds maximum representable milliseconds";
return score::cpp::make_unexpected(IConfigLoader::Error::InvalidFormat);
}
const auto result = static_cast<uint32_t>(seconds * kSecondsToMilliseconds);
if (seconds > 0.0 && result == 0U)
{
LM_LOG_ERROR() << "Sub-millisecond time value " << seconds << " seconds rounds to 0ms";
return score::cpp::make_unexpected(IConfigLoader::Error::InvalidFormat);
}
return result;
}

ApplicationType convertApplicationType(fb::ApplicationType fb_type)
{
switch (fb_type)
Expand Down Expand Up @@ -202,13 +179,7 @@ score::cpp::expected<std::optional<RestartAction>, IConfigLoader::Error> convert
{
return score::cpp::make_unexpected(delay_before_restart.error());
}
auto delay_ms = secondsToMs(*delay_before_restart);
if (!delay_ms.has_value())
{
LM_LOG_ERROR() << "Invalid value for RestartAction::delay_before_restart";
return score::cpp::make_unexpected(delay_ms.error());
}
return std::optional<RestartAction>{RestartAction{*number_of_attempts, *delay_ms}};
return std::optional<RestartAction>{RestartAction{*number_of_attempts, *delay_before_restart}};
}

std::optional<SwitchRunTargetAction> convertSwitchRunTargetAction(const fb::SwitchRunTargetAction* sa)
Expand Down Expand Up @@ -247,13 +218,7 @@ score::cpp::expected<ComponentAliveSupervision, IConfigLoader::Error> convertCom
{
return score::cpp::make_unexpected(failed_cycles_tolerance.error());
}
auto reporting_cycle_ms = secondsToMs(*reporting_cycle);
if (!reporting_cycle_ms.has_value())
{
LM_LOG_ERROR() << "Invalid value for ComponentAliveSupervision::reporting_cycle";
return score::cpp::make_unexpected(reporting_cycle_ms.error());
}
result.reporting_cycle_ms = *reporting_cycle_ms;
result.reporting_cycle_ms = *reporting_cycle;
result.failed_cycles_tolerance = *failed_cycles_tolerance;
result.min_indications = optionalScalarValue(fb_cas->min_indications());
result.max_indications = optionalScalarValue(fb_cas->max_indications());
Expand Down Expand Up @@ -428,20 +393,8 @@ score::cpp::expected<DeploymentConfig, IConfigLoader::Error> convertDeploymentCo
{
return score::cpp::make_unexpected(shutdown_timeout.error());
}
auto ready_timeout_ms = secondsToMs(*ready_timeout);
if (!ready_timeout_ms.has_value())
{
LM_LOG_ERROR() << "Invalid value for DeploymentConfig::ready_timeout";
return score::cpp::make_unexpected(ready_timeout_ms.error());
}
auto shutdown_timeout_ms = secondsToMs(*shutdown_timeout);
if (!shutdown_timeout_ms.has_value())
{
LM_LOG_ERROR() << "Invalid value for DeploymentConfig::shutdown_timeout";
return score::cpp::make_unexpected(shutdown_timeout_ms.error());
}
result.ready_timeout_ms = *ready_timeout_ms;
result.shutdown_timeout_ms = *shutdown_timeout_ms;
result.ready_timeout_ms = *ready_timeout;
result.shutdown_timeout_ms = *shutdown_timeout;
result.environmental_variables = convertEnvironmentalVariables(fb_dc->environmental_variables());
result.bin_dir = fb_dc->bin_dir()->str();
result.working_dir = fb_dc->working_dir()->str();
Expand Down Expand Up @@ -513,13 +466,7 @@ score::cpp::expected<RunTargetConfig, IConfigLoader::Error> convertRunTarget(con
result.name = fb_rt->name()->str();
result.description = safeString(fb_rt->description());
result.depends_on = convertStringVector(fb_rt->depends_on());
auto transition_timeout_ms = secondsToMs(*transition_timeout);
if (!transition_timeout_ms.has_value())
{
LM_LOG_ERROR() << "Invalid value for RunTarget::transition_timeout";
return score::cpp::make_unexpected(transition_timeout_ms.error());
}
result.transition_timeout_ms = *transition_timeout_ms;
result.transition_timeout_ms = *transition_timeout;
result.recovery_action = convertRequiredSwitchRunTargetAction(fb_rt->recovery_action());
}
return result;
Expand All @@ -539,13 +486,7 @@ score::cpp::expected<FallbackRunTargetConfig, IConfigLoader::Error> convertFallb
}
result.description = safeString(fb_frt->description());
result.depends_on = convertStringVector(fb_frt->depends_on());
auto transition_timeout_ms = secondsToMs(*transition_timeout);
if (!transition_timeout_ms.has_value())
{
LM_LOG_ERROR() << "Invalid value for FallbackRunTarget::transition_timeout";
return score::cpp::make_unexpected(transition_timeout_ms.error());
}
result.transition_timeout_ms = *transition_timeout_ms;
result.transition_timeout_ms = *transition_timeout;
}
return result;
}
Expand All @@ -562,13 +503,7 @@ score::cpp::expected<AliveSupervisionConfig, IConfigLoader::Error> convertAliveS
{
return score::cpp::make_unexpected(evaluation_cycle.error());
}
auto evaluation_cycle_ms = secondsToMs(*evaluation_cycle);
if (!evaluation_cycle_ms.has_value())
{
LM_LOG_ERROR() << "Invalid value for AliveSupervision::evaluation_cycle";
return score::cpp::make_unexpected(evaluation_cycle_ms.error());
}
return AliveSupervisionConfig{*evaluation_cycle_ms};
return AliveSupervisionConfig{*evaluation_cycle};
}

score::cpp::expected<std::optional<WatchdogConfig>, IConfigLoader::Error> convertWatchdog(const fb::Watchdog* fb_wd)
Expand Down Expand Up @@ -597,13 +532,7 @@ score::cpp::expected<std::optional<WatchdogConfig>, IConfigLoader::Error> conver
}
WatchdogConfig result{};
result.device_file_path = fb_wd->device_file_path()->str();
auto max_timeout_ms = secondsToMs(*max_timeout);
if (!max_timeout_ms.has_value())
{
LM_LOG_ERROR() << "Invalid value for Watchdog::max_timeout";
return score::cpp::make_unexpected(max_timeout_ms.error());
}
result.max_timeout_ms = *max_timeout_ms;
result.max_timeout_ms = *max_timeout;
result.deactivate_on_shutdown = *deactivate_on_shutdown;
result.require_magic_close = *require_magic_close;
return std::optional<WatchdogConfig>{std::move(result)};
Expand Down
23 changes: 9 additions & 14 deletions score/launch_manager/src/daemon/src/configuration/details/flatbuffer_type_converters.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ namespace details
template <typename TargetT>
score::cpp::expected<TargetT, IConfigLoader::Error> validateRange(int64_t value, const std::string_view field_name)
{
// Asserts ensure that std::numeric_limits<TargetT>::min() and std::numeric_limits<TargetT>::max() can be
// Asserts ensure that std::numeric_limits<TargetT>::min() and std::numeric_limits<TargetT>::max() can be
// safely cast to int64_t for the range check:
// Case 1: TargetT is unsigned and smaller than int64_t, so max fits in int64_t and min is 0
// Case 2: TargetT is signed and smaller than int64_t, so both min and max fit in int64_t
// Case 3: TargetT is signed and exactly int64_t, so min and max are the full int64_t range

static_assert(std::numeric_limits<TargetT>::is_integer, "TargetT must be an integer type");
static_assert(sizeof(TargetT) < sizeof(int64_t) ||
(sizeof(TargetT) == sizeof(int64_t) && std::is_signed_v<TargetT>),
"TargetT max must be representable as int64_t");
static_assert(
sizeof(TargetT) < sizeof(int64_t) || (sizeof(TargetT) == sizeof(int64_t) && std::is_signed_v<TargetT>),
"TargetT max must be representable as int64_t");

if (value < static_cast<int64_t>(std::numeric_limits<TargetT>::min()) ||
value > static_cast<int64_t>(std::numeric_limits<TargetT>::max()))
Expand All @@ -63,15 +63,12 @@ score::cpp::expected<TargetT, IConfigLoader::Error> validateRange(int64_t value,

// --- Scalar and enum helpers ---

/// @brief Converts a time value from seconds to milliseconds.
[[nodiscard]] score::cpp::expected<uint32_t, IConfigLoader::Error> secondsToMs(double seconds);
/// @brief Converts a FlatBuffer ApplicationType enum to the config ApplicationType.
[[nodiscard]] ApplicationType convertApplicationType(fb::ApplicationType fb_type);
/// @brief Converts a FlatBuffer ProcessState enum to the config ProcessState.
[[nodiscard]] ProcessState convertProcessState(fb::ProcessState fb_state);
/// @brief Converts a FlatBuffer SchedulingPolicy enum to a POSIX scheduling policy constant.
[[nodiscard]] score::cpp::expected<int32_t, IConfigLoader::Error> convertSchedulingPolicy(
fb::SchedulingPolicy policy);
[[nodiscard]] score::cpp::expected<int32_t, IConfigLoader::Error> convertSchedulingPolicy(fb::SchedulingPolicy policy);

// --- String and vector helpers ---

Expand All @@ -93,16 +90,15 @@ score::cpp::expected<TargetT, IConfigLoader::Error> validateRange(int64_t value,
[[nodiscard]] score::cpp::expected<std::optional<RestartAction>, IConfigLoader::Error> convertRestartAction(
const fb::RestartAction* ra);
/// @brief Converts a FlatBuffer SwitchRunTargetAction to a config SwitchRunTargetAction, or nullopt if absent.
[[nodiscard]] std::optional<SwitchRunTargetAction> convertSwitchRunTargetAction(
const fb::SwitchRunTargetAction* sa);
[[nodiscard]] std::optional<SwitchRunTargetAction> convertSwitchRunTargetAction(const fb::SwitchRunTargetAction* sa);
/// @brief Converts a required FlatBuffer SwitchRunTargetAction; asserts if null.
[[nodiscard]] SwitchRunTargetAction convertRequiredSwitchRunTargetAction(const fb::SwitchRunTargetAction* sa);

// --- Component converters ---

/// @brief Converts a FlatBuffer ComponentAliveSupervision to the config equivalent.
[[nodiscard]] score::cpp::expected<ComponentAliveSupervision, IConfigLoader::Error>
convertComponentAliveSupervision(const fb::ComponentAliveSupervision* fb_cas);
[[nodiscard]] score::cpp::expected<ComponentAliveSupervision, IConfigLoader::Error> convertComponentAliveSupervision(
const fb::ComponentAliveSupervision* fb_cas);
/// @brief Converts a FlatBuffer ApplicationProfile to the config equivalent.
[[nodiscard]] score::cpp::expected<ApplicationProfile, IConfigLoader::Error> convertApplicationProfile(
const fb::ApplicationProfile* fb_ap);
Expand All @@ -127,8 +123,7 @@ convertComponentAliveSupervision(const fb::ComponentAliveSupervision* fb_cas);
// --- Run target converters ---

/// @brief Converts a single FlatBuffer RunTarget to a RunTargetConfig.
[[nodiscard]] score::cpp::expected<RunTargetConfig, IConfigLoader::Error> convertRunTarget(
const fb::RunTarget* fb_rt);
[[nodiscard]] score::cpp::expected<RunTargetConfig, IConfigLoader::Error> convertRunTarget(const fb::RunTarget* fb_rt);
/// @brief Converts a FlatBuffer FallbackRunTarget to a FallbackRunTargetConfig.
[[nodiscard]] score::cpp::expected<FallbackRunTargetConfig, IConfigLoader::Error> convertFallbackRunTarget(
const fb::FallbackRunTarget* fb_frt);
Expand Down
Loading
Loading