Copilot commented on code in PR #13904:
URL: https://github.com/apache/trafficserver/pull/13904#discussion_r4376278064
##########
include/mgmt/config/ConfigContext.h:
##########
@@ -213,9 +218,12 @@ class ConfigContext
/// Set reload directives. Only ConfigRegistry should call this during
reload setup.
void set_reload_directives(YAML::Node node);
+ /// Supplied YAML and reload directives, shared by every copy of this
context.
Review Comment:
The comment says the payload is shared by every copy of the context, but the
setters rebind `_payload` to a new payload, so copies only share it until a
setter runs on one of them. Reword it to say copies and dependents share the
payload until a setter replaces it on one of them.
##########
src/records/unit_tests/test_ConfigRegistry.cc:
##########
@@ -309,3 +314,130 @@ TEST_CASE("ConfigRegistry is_reloadable distinguishes
handler-less entries", "[c
// An unknown key is not reloadable.
CHECK_FALSE(reg.is_reloadable("test_reloadable_unknown_xyz"));
}
+
+// ─── ConfigContext content on the execute_reload() path
───────────────────────
+//
+// No reload is in progress here, so execute_reload() hands the handler a
task-less
+// context, and the RPC-supplied content is still applied to it.
+
+namespace
+{
+ConfigContext
+reload_with(std::string const &key, YAML::Node const &content)
+{
+ // emplace/reset, not assignment: copy-assignment is what one of the tests
below checks.
Review Comment:
The reason for using `emplace`/`reset` instead of assignment is unclear, and
the comment does not say which test depends on it. Explain why assignment would
be a problem for this helper, or drop the comment if it no longer applies after
the copy-assignment change.
##########
src/records/unit_tests/test_ConfigReloadProgress.cc:
##########
@@ -1,6 +1,7 @@
/** @file
- Unit tests for the ConfigReloadProgress checker's ownership of its reload
task
+ Unit tests for the ConfigReloadProgress checker's ownership of its reload
task, and
+ for ConfigContext behaviour that needs a live reload
Review Comment:
Corrected spelling of 'behaviour' to 'behavior' to match the American
English used elsewhere in the codebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
