lordgamez commented on a change in pull request #1252:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1252#discussion_r798685117
##########
File path: docker/test/integration/features/core_functionality.feature
##########
@@ -29,3 +29,10 @@ Feature: Core flow functionalities
When the MiNiFi instance starts up
Then the Minifi logs contain the following message: "Using plaintext
FlowFileRepository" in less than 5 seconds
And the Minifi logs contain the following message: "Using plaintext
DatabaseContentRepository" in less than 1 second
+
+
+ Scenario: Processors are destructed when agent is stopped
+ Given a LogOnDestructionProcessor processor with the name
"logOnDestruction" in the "transient-minifi" flow with engine "transient-minifi"
+ When the MiNiFi instance starts up
+ Then the Minifi logs contain the following message:
"LogOnDestructionProcessor is being destructed" in less than 100 seconds
+ #TODO decrease timeout
Review comment:
I don't think timeout needs to be decreased, if it succeeds it doesn't
really matter how long it is, and in slower systems we can keep the higher
timeout values.
##########
File path: extensions/windows-event-log/tests/BookmarkTests.cpp
##########
@@ -38,8 +38,8 @@ constexpr DWORD EVT_NEXT_TIMEOUT_MS = 100;
std::unique_ptr<Bookmark> createBookmark(TestPlan &test_plan,
const std::wstring &channel,
- const utils::Identifier &uuid =
IdGenerator::getIdGenerator()->generate()) {
- const auto state_manager =
test_plan.getStateManagerProvider()->getCoreComponentStateManager(uuid);
+ const utils::Identifier &uuid,
Review comment:
Would it be maybe easier to switch the order of the arguments and keep
the default uuid? In that case we wouldn't have to generate new one manually by
hand in every test case.
##########
File path: libminifi/include/core/Repository.h
##########
@@ -118,12 +118,12 @@ class Repository : public virtual
core::SerializableComponent, public core::Trac
return found;
}
- void setConnectionMap(std::map<std::string,
std::shared_ptr<core::Connectable>> &connectionMap) {
- this->connectionMap = connectionMap;
+ void setConnectionMap(std::map<std::string, core::Connectable*>
connectionMap) {
+ this->connection_map_ = std::move(connectionMap);
}
- void setContainers(std::map<std::string, std::shared_ptr<core::Connectable>>
&containers) {
- this->containers = containers;
+ void setContainers(std::map<std::string, core::Connectable*> containers) {
+ this->containers_ = std::move(containers);
Review comment:
`this->` could also be removed
##########
File path: docker/test/integration/minifi/core/SingleNodeDockerCluster.py
##########
@@ -68,6 +68,12 @@ def acquire_container(self, name, engine='minifi-cpp',
command=None):
return self.containers.setdefault(name,
NifiContainer(self.data_directories["nifi_config_dir"], name, self.vols,
self.network, self.image_store, command))
elif engine == 'minifi-cpp':
return self.containers.setdefault(name,
MinifiContainer(self.data_directories["minifi_config_dir"], name, self.vols,
self.network, self.image_store, command))
+ elif engine == 'transient-minifi':
+ if command is None:
+ custom_command = ["/bin/sh", "-c", "cp
/tmp/minifi_config/config.yml " + MinifiContainer.MINIFI_ROOT + "/conf &&
/opt/minifi/minifi-current/bin/minifi.sh start && sleep 10 &&
/opt/minifi/minifi-current/bin/minifi.sh stop && sleep 100"]
+ else:
+ custom_command = command
+ return self.containers.setdefault(name,
MinifiContainer(self.data_directories["minifi_config_dir"], name, self.vols,
self.network, self.image_store, custom_command))
Review comment:
I would prefer creating a TransientMinifiContainer inherited from
MinifiContainer and handle the passed commands in its constructor.
##########
File path: libminifi/include/utils/GeneralUtils.h
##########
@@ -34,6 +34,16 @@ constexpr T intdiv_ceil(T numerator, T denominator) {
: numerator / denominator + (numerator % denominator != 0));
}
+/**
+ * safely converts unique_ptr from one type to another
+ * if conversion succeeds, an desired valid unique_ptr is returned, the "from"
object released and invalidated
+ * if conversion fails, an empty unique_ptr is returned, the "from" object
staying untouched
+ */
+template <typename T_To, typename T_From>
+std::unique_ptr<T_To> dynamic_unique_cast(std::unique_ptr<T_From>& obj) {
Review comment:
I'm a bit concerned about keeping the original deleter of the casted
object. Maybe we should use this proposed version:
https://stackoverflow.com/questions/21174593/downcasting-unique-ptrbase-to-unique-ptrderived?rq=1
##########
File path: libminifi/include/core/state/nodes/QueueMetrics.h
##########
@@ -57,16 +57,16 @@ class QueueMetrics : public ResponseNode {
return "QueueMetrics";
}
- void addConnection(const std::shared_ptr<minifi::Connection> &connection) {
+ void addConnection(std::unique_ptr<minifi::Connection>&& connection) {
if (nullptr != connection) {
- connections.insert(std::make_pair(connection->getName(), connection));
+ connections.insert(std::make_pair(connection->getName(),
std::move(connection)));
}
}
std::vector<SerializedResponseNode> serialize() {
std::vector<SerializedResponseNode> serialized;
- for (auto conn : connections) {
- auto connection = conn.second;
+ for (auto& conn : connections) {
+ auto& connection = conn.second;
Review comment:
I think these could be `const auto&`s
--
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]
