You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "lordgamez (via GitHub)" <gi...@apache.org> on 2025/10/31 12:40:28 UTC
[PR] MINIFICPP-2660 Handle new C2 start/stop command format [nifi-minifi-cpp]
lordgamez opened a new pull request, #2057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2057
https://issues.apache.org/jira/browse/MINIFICPP-2660
--------------------
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [ ] Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [ ] Is your initial contribution a single, squashed commit?
### For code changes:
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file?
- [ ] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2660 Handle new C2 start/stop command format [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #2057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2057#discussion_r2489823414
##########
libminifi/test/integration/C2VerifyLightweightHeartbeatAndStop.cpp:
##########
@@ -62,14 +63,23 @@ class VerifyLightWeightC2Heartbeat : public VerifyC2Base {
}
void runAssertions() override {
- REQUIRE(minifi::test::utils::verifyLogLinePresenceInPollTime(std::chrono::milliseconds(wait_time_),
+ REQUIRE(utils::verifyLogLinePresenceInPollTime(20s,
Review Comment:
With the newly added test scenarios the test runtime became longer and timed out on the slower CI machines on Windows and Mac, so I doubled the timeout and it passes now.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2660 Handle new C2 start/stop command format [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2057#discussion_r2489579404
##########
libminifi/test/libtest/integration/HTTPHandlers.h:
##########
@@ -17,7 +17,6 @@
*/
#pragma once
-// #include <algorithm>
Review Comment:
I'd remove any unused headers completely, not just comment them out.
##########
libminifi/test/libtest/integration/HTTPHandlers.cpp:
##########
@@ -453,18 +453,29 @@ void HeartbeatHandler::verifySupportedOperations(const rapidjson::Document& root
REQUIRE(operations == std::set<std::string>(magic_enum::enum_names<minifi::c2::Operation>().begin(), magic_enum::enum_names<minifi::c2::Operation>().end()));
}
-bool StoppingHeartbeatHandler::handlePost(CivetServer *, struct mg_connection *conn) {
- verify(conn);
- sendStopOperation(conn);
- return true;
-}
-void StoppingHeartbeatHandler::sendStopOperation(struct mg_connection *conn) {
- std::string resp = "{\"operation\" : \"heartbeat\", \"requested_operations\" : [{ \"operationid\" : 41, \"operation\" : \"stop\", \"operand\" : \"2438e3c8-015a-1000-79ca-83af40ec1991\" }, "
- "{ \"operationid\" : 42, \"operation\" : \"stop\", \"operand\" : \"FlowController\" } ]}";
+void StoppingHeartbeatHandler::sendStartStopOperation(struct mg_connection *conn) {
+ std::lock_guard<std::mutex> lock(post_count_mutex_);
+ std::string requested_operation;
+ if (post_count_ == 0) {
+ requested_operation = R"({ "operationid" : 41, "operation" : "stop", "operand" : "2438e3c8-015a-1000-79ca-83af40ec1991" }, )"
+ R"({ "operationid" : 42, "operation" : "stop", "operand" : "FlowController" })";
+ } else if (post_count_ == 1) {
+ requested_operation = R"({ "operationid" : 43, "operation" : "start", "operand" : "2438e3c8-015a-1000-79ca-83af40ec1991" }, )"
+ R"({ "operationid" : 44, "operation" : "start", "operand" : "FlowController" })";
+ } else if (post_count_ == 2) {
+ requested_operation = R"({ "identifier" : 45, "operation" : "STOP", "operand" : "PROCESSOR", "args" : { "processorId" : "2438e3c8-015a-1000-79ca-83af40ec1992" } }, )"
+ R"({ "identifier" : 46, "operation" : "STOP", "operand" : "FLOW" })";
+ } else if (post_count_ == 3) {
+ requested_operation = R"({ "identifier" : 47, "operation" : "START", "operand" : "PROCESSOR", "args" : { "processorId" : "2438e3c8-015a-1000-79ca-83af40ec1992" } }, )"
+ R"({ "identifier" : 48, "operation" : "START", "operand" : "FLOW" })";
+ }
+
+ std::string resp = R"({"operation" : "heartbeat", "requested_operations" : [ )" + requested_operation + " ]}";
mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
"text/plain\r\nContent-Length: %lu\r\nConnection: close\r\n\r\n",
resp.length());
mg_printf(conn, "%s", resp.c_str());
+ ++post_count_;
Review Comment:
what is this used for?
##########
libminifi/test/integration/C2VerifyLightweightHeartbeatAndStop.cpp:
##########
@@ -62,14 +63,23 @@ class VerifyLightWeightC2Heartbeat : public VerifyC2Base {
}
void runAssertions() override {
- REQUIRE(minifi::test::utils::verifyLogLinePresenceInPollTime(std::chrono::milliseconds(wait_time_),
+ REQUIRE(utils::verifyLogLinePresenceInPollTime(20s,
Review Comment:
why was this changed to hardcoded? Is it a debugging leftover?
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2660 Handle new C2 start/stop command format [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm closed pull request #2057: MINIFICPP-2660 Handle new C2 start/stop command format
URL: https://github.com/apache/nifi-minifi-cpp/pull/2057
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2660 Handle new C2 start/stop command format [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #2057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2057#discussion_r2489863561
##########
libminifi/test/libtest/integration/HTTPHandlers.cpp:
##########
@@ -453,18 +453,32 @@ void HeartbeatHandler::verifySupportedOperations(const rapidjson::Document& root
REQUIRE(operations == std::set<std::string>(magic_enum::enum_names<minifi::c2::Operation>().begin(), magic_enum::enum_names<minifi::c2::Operation>().end()));
}
-bool StoppingHeartbeatHandler::handlePost(CivetServer *, struct mg_connection *conn) {
- verify(conn);
- sendStopOperation(conn);
- return true;
-}
-void StoppingHeartbeatHandler::sendStopOperation(struct mg_connection *conn) {
- std::string resp = "{\"operation\" : \"heartbeat\", \"requested_operations\" : [{ \"operationid\" : 41, \"operation\" : \"stop\", \"operand\" : \"2438e3c8-015a-1000-79ca-83af40ec1991\" }, "
- "{ \"operationid\" : 42, \"operation\" : \"stop\", \"operand\" : \"FlowController\" } ]}";
+void StoppingHeartbeatHandler::sendStartStopOperation(struct mg_connection *conn) {
+ std::lock_guard<std::mutex> lock(post_count_mutex_);
Review Comment:
I want to keep the order of the heartbeat responses and handle the incoming heartbeats sequentially to keep the tests stable and avoid an issue like incrementing the post_count_ member twice before handling the next message thus skipping one test response. What's the downside of holding the mutex while sending data in the test scenario?
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2660 Handle new C2 start/stop command format [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #2057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2057#discussion_r2489615392
##########
libminifi/test/libtest/integration/HTTPHandlers.cpp:
##########
@@ -453,18 +453,33 @@ void HeartbeatHandler::verifySupportedOperations(const rapidjson::Document& root
REQUIRE(operations == std::set<std::string>(magic_enum::enum_names<minifi::c2::Operation>().begin(), magic_enum::enum_names<minifi::c2::Operation>().end()));
}
-bool StoppingHeartbeatHandler::handlePost(CivetServer *, struct mg_connection *conn) {
- verify(conn);
- sendStopOperation(conn);
- return true;
-}
-void StoppingHeartbeatHandler::sendStopOperation(struct mg_connection *conn) {
- std::string resp = "{\"operation\" : \"heartbeat\", \"requested_operations\" : [{ \"operationid\" : 41, \"operation\" : \"stop\", \"operand\" : \"2438e3c8-015a-1000-79ca-83af40ec1991\" }, "
- "{ \"operationid\" : 42, \"operation\" : \"stop\", \"operand\" : \"FlowController\" } ]}";
+void StoppingHeartbeatHandler::sendStartStopOperation(struct mg_connection *conn) {
+ std::lock_guard<std::mutex> lock(post_count_mutex_);
+ std::string requested_operation;
+ if (post_count_ == 0) {
+ requested_operation = R"({ "operationid" : 41, "operation" : "stop", "operand" : "2438e3c8-015a-1000-79ca-83af40ec1991" })";
+ } else if (post_count_ == 1) {
+ requested_operation = R"({ "operationid" : 42, "operation" : "stop", "operand" : "FlowController" })";
+ } else if (post_count_ == 2) {
+ requested_operation = R"({ "operationid" : 43, "operation" : "start", "operand" : "2438e3c8-015a-1000-79ca-83af40ec1991" })";
Review Comment:
I think I just started testing them separately, and added them one by one, but I think the flow and component start or stop operations can be merged to make the test run faster, I updated in https://github.com/apache/nifi-minifi-cpp/pull/2057/commits/4ce50e373a42eba19a68e33a6716bcc17468a60d
##########
libminifi/test/libtest/integration/HTTPHandlers.cpp:
##########
Review Comment:
Added negative tests in https://github.com/apache/nifi-minifi-cpp/pull/2057/commits/258d0e8f1a7d90a4221f75b4461f7f795b415399
Currently all the integration test civet server handlers are defined here, but you are right that these should be refactored and we can discuss how these should be modularized, but I think it should be addressed in a separate PR. I created a jira ticket for this issue https://issues.apache.org/jira/browse/MINIFICPP-2663
##########
libminifi/src/c2/C2Agent.cpp:
##########
@@ -871,6 +852,46 @@ void C2Agent::handle_sync(const org::apache::nifi::minifi::c2::C2ContentResponse
enqueue_c2_response(std::move(response));
}
+void C2Agent::handle_start_stop(const C2ContentResponse& resp) {
+ const auto lowered_response_name = utils::string::toLower(resp.name);
+ if (lowered_response_name == "c2") {
+ std::ignore = raise(SIGTERM);
+ }
+
+ auto executeStartStopOnComponent = [this, &resp](const std::string& component_name) {
+ update_sink_->executeOnComponent(component_name, [this, &resp, &component_name] (state::StateController& component) {
+ const std::string component_message = component_name == "FlowController" ? "all processors" : "processor " + component_name;
+ if (resp.op == Operation::stop) {
+ logger_->log_debug("Stopping {}", component_message);
+ component.stop();
+ } else {
+ logger_->log_debug("Starting {}", component_message);
+ component.start();
+ }
+ });
+ };
+
+ if (lowered_response_name == "flow" || lowered_response_name == "processor") {
+ if (lowered_response_name == "flow") {
+ executeStartStopOnComponent("FlowController");
+ } else {
+ auto processor_id = resp.getStringArgument("processorId");
+ if (processor_id) {
+ executeStartStopOnComponent(processor_id.value());
+ } else {
+ logger_->log_warn("Processor start/stop request missing 'processorId' argument");
+ }
+ }
+ } else {
+ executeStartStopOnComponent(resp.name);
+ }
Review Comment:
Updated in https://github.com/apache/nifi-minifi-cpp/pull/2057/commits/4ce50e373a42eba19a68e33a6716bcc17468a60d
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2660 Handle new C2 start/stop command format [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2057#discussion_r2489680295
##########
libminifi/test/libtest/integration/HTTPHandlers.cpp:
##########
@@ -453,18 +453,32 @@ void HeartbeatHandler::verifySupportedOperations(const rapidjson::Document& root
REQUIRE(operations == std::set<std::string>(magic_enum::enum_names<minifi::c2::Operation>().begin(), magic_enum::enum_names<minifi::c2::Operation>().end()));
}
-bool StoppingHeartbeatHandler::handlePost(CivetServer *, struct mg_connection *conn) {
- verify(conn);
- sendStopOperation(conn);
- return true;
-}
-void StoppingHeartbeatHandler::sendStopOperation(struct mg_connection *conn) {
- std::string resp = "{\"operation\" : \"heartbeat\", \"requested_operations\" : [{ \"operationid\" : 41, \"operation\" : \"stop\", \"operand\" : \"2438e3c8-015a-1000-79ca-83af40ec1991\" }, "
- "{ \"operationid\" : 42, \"operation\" : \"stop\", \"operand\" : \"FlowController\" } ]}";
+void StoppingHeartbeatHandler::sendStartStopOperation(struct mg_connection *conn) {
+ std::lock_guard<std::mutex> lock(post_count_mutex_);
Review Comment:
If this is only used to protect the post_count_ member, then we should not hold the mutex while sending data over the network. Maybe the read and the increment should be separate steps in separate critical sections.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2660 Handle new C2 start/stop command format [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2057#discussion_r2489223370
##########
libminifi/src/c2/C2Agent.cpp:
##########
@@ -871,6 +852,46 @@ void C2Agent::handle_sync(const org::apache::nifi::minifi::c2::C2ContentResponse
enqueue_c2_response(std::move(response));
}
+void C2Agent::handle_start_stop(const C2ContentResponse& resp) {
+ const auto lowered_response_name = utils::string::toLower(resp.name);
+ if (lowered_response_name == "c2") {
+ std::ignore = raise(SIGTERM);
+ }
+
+ auto executeStartStopOnComponent = [this, &resp](const std::string& component_name) {
+ update_sink_->executeOnComponent(component_name, [this, &resp, &component_name] (state::StateController& component) {
+ const std::string component_message = component_name == "FlowController" ? "all processors" : "processor " + component_name;
+ if (resp.op == Operation::stop) {
+ logger_->log_debug("Stopping {}", component_message);
+ component.stop();
+ } else {
+ logger_->log_debug("Starting {}", component_message);
+ component.start();
+ }
+ });
+ };
+
+ if (lowered_response_name == "flow" || lowered_response_name == "processor") {
+ if (lowered_response_name == "flow") {
+ executeStartStopOnComponent("FlowController");
+ } else {
+ auto processor_id = resp.getStringArgument("processorId");
+ if (processor_id) {
+ executeStartStopOnComponent(processor_id.value());
+ } else {
+ logger_->log_warn("Processor start/stop request missing 'processorId' argument");
+ }
+ }
+ } else {
+ executeStartStopOnComponent(resp.name);
+ }
Review Comment:
```suggestion
if (lowered_response_name == "flow" ) {
executeStartStopOnComponent("FlowController");
} else if (lowered_response_name == "processor") {
auto processor_id = resp.getStringArgument("processorId");
if (processor_id) {
executeStartStopOnComponent(processor_id.value());
} else {
logger_->log_warn("Processor start/stop request missing 'processorId' argument");
}
} else {
executeStartStopOnComponent(resp.name);
}
```
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2660 Handle new C2 start/stop command format [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2057#discussion_r2489615274
##########
libminifi/test/libtest/integration/HTTPHandlers.cpp:
##########
@@ -453,18 +453,29 @@ void HeartbeatHandler::verifySupportedOperations(const rapidjson::Document& root
REQUIRE(operations == std::set<std::string>(magic_enum::enum_names<minifi::c2::Operation>().begin(), magic_enum::enum_names<minifi::c2::Operation>().end()));
}
-bool StoppingHeartbeatHandler::handlePost(CivetServer *, struct mg_connection *conn) {
- verify(conn);
- sendStopOperation(conn);
- return true;
-}
-void StoppingHeartbeatHandler::sendStopOperation(struct mg_connection *conn) {
- std::string resp = "{\"operation\" : \"heartbeat\", \"requested_operations\" : [{ \"operationid\" : 41, \"operation\" : \"stop\", \"operand\" : \"2438e3c8-015a-1000-79ca-83af40ec1991\" }, "
- "{ \"operationid\" : 42, \"operation\" : \"stop\", \"operand\" : \"FlowController\" } ]}";
+void StoppingHeartbeatHandler::sendStartStopOperation(struct mg_connection *conn) {
+ std::lock_guard<std::mutex> lock(post_count_mutex_);
+ std::string requested_operation;
+ if (post_count_ == 0) {
+ requested_operation = R"({ "operationid" : 41, "operation" : "stop", "operand" : "2438e3c8-015a-1000-79ca-83af40ec1991" }, )"
+ R"({ "operationid" : 42, "operation" : "stop", "operand" : "FlowController" })";
+ } else if (post_count_ == 1) {
+ requested_operation = R"({ "operationid" : 43, "operation" : "start", "operand" : "2438e3c8-015a-1000-79ca-83af40ec1991" }, )"
+ R"({ "operationid" : 44, "operation" : "start", "operand" : "FlowController" })";
+ } else if (post_count_ == 2) {
+ requested_operation = R"({ "identifier" : 45, "operation" : "STOP", "operand" : "PROCESSOR", "args" : { "processorId" : "2438e3c8-015a-1000-79ca-83af40ec1992" } }, )"
+ R"({ "identifier" : 46, "operation" : "STOP", "operand" : "FLOW" })";
+ } else if (post_count_ == 3) {
+ requested_operation = R"({ "identifier" : 47, "operation" : "START", "operand" : "PROCESSOR", "args" : { "processorId" : "2438e3c8-015a-1000-79ca-83af40ec1992" } }, )"
+ R"({ "identifier" : 48, "operation" : "START", "operand" : "FLOW" })";
+ }
+
+ std::string resp = R"({"operation" : "heartbeat", "requested_operations" : [ )" + requested_operation + " ]}";
mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
"text/plain\r\nContent-Length: %lu\r\nConnection: close\r\n\r\n",
resp.length());
mg_printf(conn, "%s", resp.c_str());
+ ++post_count_;
Review Comment:
what is this used for?
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2660 Handle new C2 start/stop command format [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on PR #2057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2057#issuecomment-3472935614
It can still be a topic of discussion whether we should keep the handling of the old format in case any C2 server implementation uses that for starting/stopping components, or if we should remove that entirely?
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2660 Handle new C2 start/stop command format [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2057#discussion_r2489204653
##########
libminifi/test/libtest/integration/HTTPHandlers.cpp:
##########
Review Comment:
We should also have some negative tests, and I'm not sure this logic belongs in the library.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2660 Handle new C2 start/stop command format [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #2057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2057#discussion_r2495278351
##########
libminifi/test/libtest/integration/HTTPHandlers.cpp:
##########
@@ -453,18 +453,32 @@ void HeartbeatHandler::verifySupportedOperations(const rapidjson::Document& root
REQUIRE(operations == std::set<std::string>(magic_enum::enum_names<minifi::c2::Operation>().begin(), magic_enum::enum_names<minifi::c2::Operation>().end()));
}
-bool StoppingHeartbeatHandler::handlePost(CivetServer *, struct mg_connection *conn) {
- verify(conn);
- sendStopOperation(conn);
- return true;
-}
-void StoppingHeartbeatHandler::sendStopOperation(struct mg_connection *conn) {
- std::string resp = "{\"operation\" : \"heartbeat\", \"requested_operations\" : [{ \"operationid\" : 41, \"operation\" : \"stop\", \"operand\" : \"2438e3c8-015a-1000-79ca-83af40ec1991\" }, "
- "{ \"operationid\" : 42, \"operation\" : \"stop\", \"operand\" : \"FlowController\" } ]}";
+void StoppingHeartbeatHandler::sendStartStopOperation(struct mg_connection *conn) {
+ std::lock_guard<std::mutex> lock(post_count_mutex_);
Review Comment:
Good point, that naming can be confusing. Updated in https://github.com/apache/nifi-minifi-cpp/pull/2057/commits/dc9b048f7bfeb515519b3eeedf294c9b68c46214
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2660 Handle new C2 start/stop command format [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #2057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2057#discussion_r2489828803
##########
libminifi/test/libtest/integration/HTTPHandlers.h:
##########
@@ -17,7 +17,6 @@
*/
#pragma once
-// #include <algorithm>
Review Comment:
It was removed in https://github.com/apache/nifi-minifi-cpp/pull/2057/commits/4ce50e373a42eba19a68e33a6716bcc17468a60d
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2660 Handle new C2 start/stop command format [nifi-minifi-cpp]
Posted by "adamdebreceni (via GitHub)" <gi...@apache.org>.
adamdebreceni commented on code in PR #2057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2057#discussion_r2489202900
##########
libminifi/test/libtest/integration/HTTPHandlers.cpp:
##########
@@ -453,18 +453,33 @@ void HeartbeatHandler::verifySupportedOperations(const rapidjson::Document& root
REQUIRE(operations == std::set<std::string>(magic_enum::enum_names<minifi::c2::Operation>().begin(), magic_enum::enum_names<minifi::c2::Operation>().end()));
}
-bool StoppingHeartbeatHandler::handlePost(CivetServer *, struct mg_connection *conn) {
- verify(conn);
- sendStopOperation(conn);
- return true;
-}
-void StoppingHeartbeatHandler::sendStopOperation(struct mg_connection *conn) {
- std::string resp = "{\"operation\" : \"heartbeat\", \"requested_operations\" : [{ \"operationid\" : 41, \"operation\" : \"stop\", \"operand\" : \"2438e3c8-015a-1000-79ca-83af40ec1991\" }, "
- "{ \"operationid\" : 42, \"operation\" : \"stop\", \"operand\" : \"FlowController\" } ]}";
+void StoppingHeartbeatHandler::sendStartStopOperation(struct mg_connection *conn) {
+ std::lock_guard<std::mutex> lock(post_count_mutex_);
+ std::string requested_operation;
+ if (post_count_ == 0) {
+ requested_operation = R"({ "operationid" : 41, "operation" : "stop", "operand" : "2438e3c8-015a-1000-79ca-83af40ec1991" })";
+ } else if (post_count_ == 1) {
+ requested_operation = R"({ "operationid" : 42, "operation" : "stop", "operand" : "FlowController" })";
+ } else if (post_count_ == 2) {
+ requested_operation = R"({ "operationid" : 43, "operation" : "start", "operand" : "2438e3c8-015a-1000-79ca-83af40ec1991" })";
Review Comment:
previously we sent both operations in a single request, what prompted the change?
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2660 Handle new C2 start/stop command format [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2057#discussion_r2495219389
##########
libminifi/test/libtest/integration/HTTPHandlers.cpp:
##########
@@ -453,18 +453,32 @@ void HeartbeatHandler::verifySupportedOperations(const rapidjson::Document& root
REQUIRE(operations == std::set<std::string>(magic_enum::enum_names<minifi::c2::Operation>().begin(), magic_enum::enum_names<minifi::c2::Operation>().end()));
}
-bool StoppingHeartbeatHandler::handlePost(CivetServer *, struct mg_connection *conn) {
- verify(conn);
- sendStopOperation(conn);
- return true;
-}
-void StoppingHeartbeatHandler::sendStopOperation(struct mg_connection *conn) {
- std::string resp = "{\"operation\" : \"heartbeat\", \"requested_operations\" : [{ \"operationid\" : 41, \"operation\" : \"stop\", \"operand\" : \"2438e3c8-015a-1000-79ca-83af40ec1991\" }, "
- "{ \"operationid\" : 42, \"operation\" : \"stop\", \"operand\" : \"FlowController\" } ]}";
+void StoppingHeartbeatHandler::sendStartStopOperation(struct mg_connection *conn) {
+ std::lock_guard<std::mutex> lock(post_count_mutex_);
Review Comment:
I got confused by the name: it's not really protecting post_count_ then, but also ensures in order handling of heartbeats. I think I would handle this differently (with a state machine), but I can't come up with a good reason why this is bad, so I'm not objecting to using a mutex for this purpose. But its name should be changed, since it managed to confuse me, and I find this control flow surprising, so I think a longer description in a code comment would benefit future readers, about how this handler works, and what purposes the mutex serves.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org