exceptionfactory commented on code in PR #8123:
URL: https://github.com/apache/nifi/pull/8123#discussion_r1416185237
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/pom.xml:
##########
@@ -108,7 +108,7 @@
<nifi.provenance.repository.buffer.size>100000</nifi.provenance.repository.buffer.size>
<!-- Component status repository properties -->
-
<nifi.components.status.repository.implementation>org.apache.nifi.controller.status.history.VolatileComponentStatusRepository</nifi.components.status.repository.implementation>
+
<nifi.components.status.repository.implementation>org.apache.nifi.controller.status.history.questdb.EmbeddedQuestDbStatusHistoryRepository</nifi.components.status.repository.implementation>
Review Comment:
I recommend reverting this particular change for this pull request. There is
a good deal to review, so if we get to the place where this is ready to be the
default setting, we should consider that after reviewing the substantive
changes.
##########
nifi-api/src/main/java/org/apache/nifi/controller/status/ConnectionStatus.java:
##########
@@ -22,6 +22,7 @@
/**
*/
public class ConnectionStatus implements Cloneable {
+ private long createdAtInMs;
Review Comment:
This addition raises several questions and concerns. It does not seem like
the NiFi API definition should include the creation timestamp of components in
all circumstances. It would make much more sense to track this as a feature
internal to the framework.
As far as naming the `InMs` suffix is awkward, and it would be much better
to use `java.time.Instant` for model classes.
--
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]
