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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to