Skip to content

Commit 3ee41e4

Browse files
authored
ChronoKVS: connect to remote visors via a config file (#470) (#628) (#652)
* feat(chronokvs): support config file path in constructors (#470) ChronoKVS was hardcoded to the ChronoLog client defaults (localhost:5555), limiting it to local deployments. Add an optional config_path parameter to ChronoKVS, ChronoKVSMapper and ChronoKVSClientAdapter constructors so callers can point at a ChronoLog client JSON configuration and connect to remote deployments. Default-constructed instances keep their previous behavior. The adapter loads the file via ClientConfiguration::load_from_file() before connecting and throws std::runtime_error if loading fails, matching the acceptance criteria in the issue. The writer/reader examples accept --config/-c (parse_conf_path_arg) so the plumbing can be exercised end-to-end. Adds a unit test that verifies the constructor throws on missing or section-less config files without requiring a running ChronoLog deployment. * ci(local-pipeline): sanitize branch name for artifact uploads actions/upload-artifact rejects names containing '/'. Branch names like 'feature/470-...' caused the ctest-results upload step to fail. Compute a slugified branch name (slashes replaced with dashes) once and reuse it for both artifact uploads and the job summary references. * refactor(chronokvs): rename confManager to client_config Per review feedback on PR #628: confManager is a remnant of the old ConfigurationManager class. Use a clearer client-facing name. * refactor(chronokvs): replace throwing constructors with noexcept Create() factory Per review feedback on PR #628 (Inna): throwing exceptions across a client-library boundary forces callers to wrap construction in try/catch without an obvious set of exception types to catch. Convert the public ChronoKVS API to a factory pattern instead: - Make ChronoKVS constructors private. - Add static std::unique_ptr<ChronoKVS> Create(...) noexcept overloads that catch any construction-path exception, log it via the configured log level, and return nullptr to signal failure. - Update examples, integration test, and unit tests to use the factory and check for nullptr instead of catching std::runtime_error. Internal layers (ChronoKVSMapper, ChronoKVSClientAdapter) keep using exceptions for clarity; they are now caught at the library boundary inside Create() rather than escaping to user code.
1 parent 4f5ac90 commit 3ee41e4

13 files changed

Lines changed: 277 additions & 51 deletions

.github/workflows/local-pipeline.yml

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ jobs:
3636
with:
3737
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.ref }}
3838

39+
- name: Compute sanitized branch slug
40+
id: branch-slug
41+
run: |
42+
RAW="${{ github.head_ref || github.ref_name }}"
43+
SAFE="${RAW//\//-}"
44+
echo "slug=${SAFE}" >> "$GITHUB_OUTPUT"
45+
3946
- name: Set up Docker Buildx
4047
uses: docker/setup-buildx-action@v4
4148

@@ -372,7 +379,7 @@ jobs:
372379
if: always()
373380
uses: actions/upload-artifact@v7
374381
with:
375-
name: ctest-results-${{ github.head_ref || github.ref_name }}
382+
name: ctest-results-${{ steps.branch-slug.outputs.slug }}
376383
path: ./ctest-results
377384
retention-days: 14
378385

@@ -439,7 +446,7 @@ jobs:
439446
- name: Upload chronolog-install artifact
440447
uses: actions/upload-artifact@v7
441448
with:
442-
name: chronolog-install-${{ github.head_ref || github.ref_name }}
449+
name: chronolog-install-${{ steps.branch-slug.outputs.slug }}
443450
path: ./chronolog-install
444451
retention-days: 7
445452

@@ -484,6 +491,7 @@ jobs:
484491
)
485492
486493
BRANCH="${{ github.head_ref || github.ref_name }}"
494+
BRANCH_SLUG="${{ steps.branch-slug.outputs.slug }}"
487495
488496
{
489497
echo "## 🧪 ChronoLog Local Pipeline"
@@ -519,13 +527,13 @@ jobs:
519527
echo "</details>"
520528
echo ""
521529
fi
522-
echo "_ctest is non-gating while the suite is being refactored. Full logs in the_ \`ctest-results-${BRANCH}\` _artifact._"
530+
echo "_ctest is non-gating while the suite is being refactored. Full logs in the_ \`ctest-results-${BRANCH_SLUG}\` _artifact._"
523531
else
524-
echo "⚠️ No ctest results captured (see \`ctest-results-${BRANCH}\` artifact)."
532+
echo "⚠️ No ctest results captured (see \`ctest-results-${BRANCH_SLUG}\` artifact)."
525533
fi
526534
echo ""
527535
echo "### 📦 Artifacts"
528536
echo ""
529-
echo "- \`chronolog-install-${BRANCH}\` — Release install tree with test binaries"
530-
echo "- \`ctest-results-${BRANCH}\` — ctest JUnit XML, log, and \`Testing/\` directory"
537+
echo "- \`chronolog-install-${BRANCH_SLUG}\` — Release install tree with test binaries"
538+
echo "- \`ctest-results-${BRANCH_SLUG}\` — ctest JUnit XML, log, and \`Testing/\` directory"
531539
} >> "$GITHUB_STEP_SUMMARY"

plugins/chrono-kvs/examples/CMakeLists.txt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,16 @@ add_executable(chronokvs_reader_example chronokvs_reader_example.cpp)
55
target_link_libraries(chronokvs_writer_example PRIVATE chronokvs)
66
target_link_libraries(chronokvs_reader_example PRIVATE chronokvs)
77

8+
# cmd_arg_parse.h lives in chronolog_client's public include dir.
9+
# chronokvs consumes chronolog_client privately, so pull its include
10+
# interface in directly here for the examples that use the helper.
11+
target_include_directories(chronokvs_writer_example PRIVATE
12+
$<TARGET_PROPERTY:chronolog_client,INTERFACE_INCLUDE_DIRECTORIES>
13+
)
14+
target_include_directories(chronokvs_reader_example PRIVATE
15+
$<TARGET_PROPERTY:chronolog_client,INTERFACE_INCLUDE_DIRECTORIES>
16+
)
17+
818
set_target_properties(chronokvs_writer_example PROPERTIES
919
OUTPUT_NAME chrono-chronokvs-example-writer
1020
BUILD_WITH_INSTALL_RPATH TRUE

plugins/chrono-kvs/examples/chronokvs_reader_example.cpp

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include <iostream>
55
#include <fstream>
66
#include <vector>
7+
#include <cmd_arg_parse.h>
78
#include "chronokvs.h"
89

910
std::vector<std::uint64_t> readTimestampsFromFile(const std::string& filename)
@@ -24,10 +25,20 @@ std::vector<std::uint64_t> readTimestampsFromFile(const std::string& filename)
2425
return timestamps;
2526
}
2627

27-
int main()
28+
int main(int argc, char** argv)
2829
{
29-
// Create ChronoKVS instance
30-
chronokvs::ChronoKVS chronoKVS;
30+
// Optional ChronoLog client config file (-c/--config). When omitted,
31+
// ChronoKVS uses the built-in defaults (localhost deployment).
32+
std::string conf_file_path = parse_conf_path_arg(argc, argv);
33+
34+
// Create ChronoKVS instance: with config file when provided, otherwise defaults.
35+
auto chronoKVS =
36+
conf_file_path.empty() ? chronokvs::ChronoKVS::Create() : chronokvs::ChronoKVS::Create(conf_file_path);
37+
if(!chronoKVS)
38+
{
39+
std::cerr << "Failed to initialize ChronoKVS\n";
40+
return 1;
41+
}
3142

3243
// Read timestamps from file
3344
std::vector<std::uint64_t> timestamps = readTimestampsFromFile("chronokvs_timestamps.txt");
@@ -48,7 +59,7 @@ int main()
4859

4960
// Read and display history for key2
5061
std::cout << "\nReading history for key2:\n";
51-
auto historyForKey2 = chronoKVS.get_history(key2);
62+
auto historyForKey2 = chronoKVS->get_history(key2);
5263

5364
if(historyForKey2.empty())
5465
{
@@ -65,8 +76,8 @@ int main()
6576

6677
// Read specific values for key2 at both timestamps
6778
std::cout << "Reading specific values for key2:\n";
68-
std::string value2_t1 = chronoKVS.get(key2, timestamps[1]);
69-
std::string value2_t2 = chronoKVS.get(key2, timestamps[2]);
79+
std::string value2_t1 = chronoKVS->get(key2, timestamps[1]);
80+
std::string value2_t2 = chronoKVS->get(key2, timestamps[2]);
7081

7182
std::cout << "Value at timestamp " << timestamps[1] << ": " << (value2_t1.empty() ? "Not found" : value2_t1)
7283
<< "\n";
@@ -75,7 +86,7 @@ int main()
7586

7687
// Read and display value for key1
7788
std::cout << "\nReading value for key1 at timestamp " << timestamps[0] << ":\n";
78-
std::string value1 = chronoKVS.get(key1, timestamps[0]);
89+
std::string value1 = chronoKVS->get(key1, timestamps[0]);
7990
if(!value1.empty())
8091
{
8192
std::cout << "Value: " << value1 << "\n";
@@ -87,7 +98,7 @@ int main()
8798

8899
// Read and display events in a time range for key2
89100
std::cout << "\nReading events for key2 in time range [" << timestamps[1] << ", " << timestamps[2] + 1 << "):\n";
90-
auto rangeEvents = chronoKVS.get_range(key2, timestamps[1], timestamps[2] + 1);
101+
auto rangeEvents = chronoKVS->get_range(key2, timestamps[1], timestamps[2] + 1);
91102
if(rangeEvents.empty())
92103
{
93104
std::cout << "No events found in the specified time range\n";
@@ -103,7 +114,7 @@ int main()
103114

104115
// Read and display the earliest event for key2
105116
std::cout << "Reading earliest event for key2:\n";
106-
auto earliestEvent = chronoKVS.get_earliest(key2);
117+
auto earliestEvent = chronoKVS->get_earliest(key2);
107118
if(earliestEvent.has_value())
108119
{
109120
std::cout << "Earliest event - Timestamp: " << earliestEvent->timestamp
@@ -116,7 +127,7 @@ int main()
116127

117128
// Read and display the latest event for key2
118129
std::cout << "\nReading latest event for key2:\n";
119-
auto latestEvent = chronoKVS.get_latest(key2);
130+
auto latestEvent = chronoKVS->get_latest(key2);
120131
if(latestEvent.has_value())
121132
{
122133
std::cout << "Latest event - Timestamp: " << latestEvent->timestamp << "\nValue : " << latestEvent->value

plugins/chrono-kvs/examples/chronokvs_writer_example.cpp

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,23 @@
22
#include <string>
33
#include <iostream>
44
#include <fstream>
5+
#include <cmd_arg_parse.h>
56
#include "chronokvs.h"
67

7-
int main()
8+
int main(int argc, char** argv)
89
{
9-
// Create ChronoKVS instance
10-
chronokvs::ChronoKVS chronoKVS;
10+
// Optional ChronoLog client config file (-c/--config). When omitted,
11+
// ChronoKVS uses the built-in defaults (localhost deployment).
12+
std::string conf_file_path = parse_conf_path_arg(argc, argv);
13+
14+
// Create ChronoKVS instance: with config file when provided, otherwise defaults.
15+
auto chronoKVS =
16+
conf_file_path.empty() ? chronokvs::ChronoKVS::Create() : chronokvs::ChronoKVS::Create(conf_file_path);
17+
if(!chronoKVS)
18+
{
19+
std::cerr << "Failed to initialize ChronoKVS\n";
20+
return 1;
21+
}
1122

1223
// Define key-value pairs
1324
std::string key1 = "key1";
@@ -18,9 +29,9 @@ int main()
1829

1930
// Insert key-value pairs and store timestamps
2031
std::cout << "Putting key-value pairs into ChronoKVS...\n";
21-
std::uint64_t timestamp1 = chronoKVS.put(key1, value1);
22-
std::uint64_t timestamp2 = chronoKVS.put(key2, value2);
23-
std::uint64_t timestamp3 = chronoKVS.put(key2, value3);
32+
std::uint64_t timestamp1 = chronoKVS->put(key1, value1);
33+
std::uint64_t timestamp2 = chronoKVS->put(key2, value2);
34+
std::uint64_t timestamp3 = chronoKVS->put(key2, value3);
2435

2536
std::cout << "Inserted values with timestamps:\n";
2637
std::cout << "1. key1=" << value1 << " timestamp=" << timestamp1 << "\n";

plugins/chrono-kvs/include/chronokvs.h

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,52 @@ class ChronoKVS
2222
std::unique_ptr<ChronoKVSMapper> mapper;
2323
LogLevel logLevel_;
2424

25+
// Private constructors. May throw on configuration or connection failure;
26+
// the public Create() factories catch those exceptions at the library
27+
// boundary and signal failure via a nullptr return.
28+
explicit ChronoKVS(LogLevel level);
29+
explicit ChronoKVS(const std::string& config_path, LogLevel level);
30+
2531
public:
2632
/**
27-
* @brief Construct a ChronoKVS instance with optional log level
28-
* @param level The logging level to use. Default is DEBUG in debug builds, ERROR in release builds.
33+
* @brief Create a ChronoKVS instance using built-in default ChronoLog
34+
* client configuration (localhost deployment).
35+
*
36+
* This factory does not propagate exceptions across the library boundary:
37+
* any failure during configuration loading or ChronoLog connection is
38+
* logged at the configured @p level and signalled by returning nullptr.
39+
*
40+
* @param level
41+
* The logging level to use. Default is DEBUG in debug builds, ERROR in release builds.
42+
*
43+
* @return std::unique_ptr<ChronoKVS>
44+
* A connected ChronoKVS instance, or nullptr if construction failed.
45+
*/
46+
static std::unique_ptr<ChronoKVS> Create(LogLevel level = getDefaultLogLevel()) noexcept;
47+
48+
/**
49+
* @brief Create a ChronoKVS instance using a ChronoLog client configuration file.
50+
*
51+
* Loads the JSON configuration at @p config_path and uses the resulting portal,
52+
* query and logging settings to connect to ChronoLog. Pass an empty string to
53+
* fall back to the built-in defaults (localhost deployment).
54+
*
55+
* This factory does not propagate exceptions across the library boundary:
56+
* any failure during configuration loading or ChronoLog connection is
57+
* logged at the configured @p level and signalled by returning nullptr.
58+
*
59+
* @param config_path
60+
* Path to a ChronoLog client configuration JSON file. Empty means
61+
* "use defaults".
62+
* @param level
63+
* The logging level to use. Default is DEBUG in debug builds, ERROR in release builds.
64+
*
65+
* @return std::unique_ptr<ChronoKVS>
66+
* A connected ChronoKVS instance, or nullptr if @p config_path could
67+
* not be loaded or the ChronoLog connection failed.
2968
*/
30-
explicit ChronoKVS(LogLevel level = getDefaultLogLevel());
69+
static std::unique_ptr<ChronoKVS> Create(const std::string& config_path,
70+
LogLevel level = getDefaultLogLevel()) noexcept;
3171

3272
/**
3373
* @brief Get the current log level

plugins/chrono-kvs/src/chronokvs.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,47 @@ ChronoKVS::ChronoKVS(LogLevel level)
1515
, logLevel_(level)
1616
{}
1717

18+
ChronoKVS::ChronoKVS(const std::string& config_path, LogLevel level)
19+
: mapper(std::make_unique<ChronoKVSMapper>(config_path, level))
20+
, logLevel_(level)
21+
{}
22+
23+
std::unique_ptr<ChronoKVS> ChronoKVS::Create(LogLevel level) noexcept
24+
{
25+
try
26+
{
27+
return std::unique_ptr<ChronoKVS>(new ChronoKVS(level));
28+
}
29+
catch(const std::exception& e)
30+
{
31+
CHRONOKVS_ERROR(level, "ChronoKVS construction failed: ", e.what());
32+
return nullptr;
33+
}
34+
catch(...)
35+
{
36+
CHRONOKVS_ERROR(level, "ChronoKVS construction failed: unknown exception");
37+
return nullptr;
38+
}
39+
}
40+
41+
std::unique_ptr<ChronoKVS> ChronoKVS::Create(const std::string& config_path, LogLevel level) noexcept
42+
{
43+
try
44+
{
45+
return std::unique_ptr<ChronoKVS>(new ChronoKVS(config_path, level));
46+
}
47+
catch(const std::exception& e)
48+
{
49+
CHRONOKVS_ERROR(level, "ChronoKVS construction failed (config_path='", config_path, "'): ", e.what());
50+
return nullptr;
51+
}
52+
catch(...)
53+
{
54+
CHRONOKVS_ERROR(level, "ChronoKVS construction failed (config_path='", config_path, "'): unknown exception");
55+
return nullptr;
56+
}
57+
}
58+
1859
ChronoKVS::~ChronoKVS() = default;
1960

2061
std::uint64_t ChronoKVS::put(const std::string& key, const std::string& value)

plugins/chrono-kvs/src/chronokvs_client_adapter.cpp

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,38 @@ static int DEFAULT_FLAGS = 0;
1818
ChronoKVSClientAdapter::ChronoKVSClientAdapter(LogLevel level)
1919
: logLevel_(level)
2020
{
21-
chronolog::ClientConfiguration confManager;
22-
23-
// Configure portal and query services from the configuration manager
24-
chronolog::ClientPortalServiceConf portalConf{confManager.PORTAL_CONF.PROTO_CONF,
25-
confManager.PORTAL_CONF.IP,
26-
confManager.PORTAL_CONF.PORT,
27-
confManager.PORTAL_CONF.PROVIDER_ID};
28-
29-
chronolog::ClientQueryServiceConf queryConf{confManager.QUERY_CONF.PROTO_CONF,
30-
confManager.QUERY_CONF.IP,
31-
confManager.QUERY_CONF.PORT,
32-
confManager.QUERY_CONF.PROVIDER_ID};
21+
chronolog::ClientConfiguration client_config;
22+
initialize(client_config);
23+
}
24+
25+
ChronoKVSClientAdapter::ChronoKVSClientAdapter(const std::string& config_path, LogLevel level)
26+
: logLevel_(level)
27+
{
28+
chronolog::ClientConfiguration client_config;
29+
if(!config_path.empty())
30+
{
31+
CHRONOKVS_INFO(logLevel_, "Loading ChronoLog client configuration from '", config_path, "'");
32+
if(!client_config.load_from_file(config_path))
33+
{
34+
CHRONOKVS_ERROR(logLevel_, "Failed to load configuration file: ", config_path);
35+
throw std::runtime_error("Failed to load config file: " + config_path);
36+
}
37+
}
38+
initialize(client_config);
39+
}
40+
41+
void ChronoKVSClientAdapter::initialize(const chronolog::ClientConfiguration& client_config)
42+
{
43+
// Configure portal and query services from the client configuration
44+
chronolog::ClientPortalServiceConf portalConf{client_config.PORTAL_CONF.PROTO_CONF,
45+
client_config.PORTAL_CONF.IP,
46+
client_config.PORTAL_CONF.PORT,
47+
client_config.PORTAL_CONF.PROVIDER_ID};
48+
49+
chronolog::ClientQueryServiceConf queryConf{client_config.QUERY_CONF.PROTO_CONF,
50+
client_config.QUERY_CONF.IP,
51+
client_config.QUERY_CONF.PORT,
52+
client_config.QUERY_CONF.PROVIDER_ID};
3353

3454
// Initialize and connect the ChronoLog client
3555
CHRONOKVS_INFO(logLevel_, "Connecting to ChronoLog at ", portalConf.IP, ":", portalConf.PORT);

plugins/chrono-kvs/src/chronokvs_client_adapter.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@
1111

1212
#include <chronolog_client.h>
1313

14+
namespace chronolog
15+
{
16+
class ClientConfiguration;
17+
}
18+
1419
#include "chronokvs_types.h"
1520
#include "chronokvs_logger.h"
1621

@@ -37,9 +42,15 @@ class ChronoKVSClientAdapter
3742
// Helper method to release and remove a cached handle for a key (before reads)
3843
void flushCachedHandle(const std::string& key);
3944

45+
// Connect to ChronoLog using the already-populated configuration and ensure
46+
// the default chronicle exists. Used by all constructors.
47+
void initialize(const chronolog::ClientConfiguration& client_config);
48+
4049
public:
4150
explicit ChronoKVSClientAdapter(LogLevel level);
4251

52+
explicit ChronoKVSClientAdapter(const std::string& config_path, LogLevel level);
53+
4354
~ChronoKVSClientAdapter();
4455

4556
std::uint64_t storeEvent(const std::string& key, const std::string& value);

plugins/chrono-kvs/src/chronokvs_mapper.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@ ChronoKVSMapper::ChronoKVSMapper(LogLevel level)
2020
chronoClientAdapter = std::make_unique<ChronoKVSClientAdapter>(level);
2121
}
2222

23+
ChronoKVSMapper::ChronoKVSMapper(const std::string& config_path, LogLevel level)
24+
: logLevel_(level)
25+
{
26+
chronoClientAdapter = std::make_unique<ChronoKVSClientAdapter>(config_path, level);
27+
}
28+
2329
std::uint64_t ChronoKVSMapper::storeKeyValue(const std::string& key, const std::string& value)
2430
{
2531
// Input validation

0 commit comments

Comments
 (0)