bejancsaba commented on code in PR #6281:
URL: https://github.com/apache/nifi/pull/6281#discussion_r955386864
##########
c2/c2-protocol/c2-protocol-api/pom.xml:
##########
@@ -33,5 +33,9 @@ limitations under the License.
<artifactId>c2-protocol-component-api</artifactId>
<version>1.18.0-SNAPSHOT</version>
</dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
Review Comment:
Thank you!
##########
c2/c2-client-bundle/c2-client-service/src/main/java/org/apache/nifi/c2/client/service/C2ClientService.java:
##########
@@ -41,8 +41,8 @@ public C2ClientService(C2Client client, C2HeartbeatFactory
c2HeartbeatFactory, C
}
public void sendHeartbeat(RuntimeInfoWrapper runtimeInfoWrapper) {
- C2Heartbeat c2Heartbeat =
c2HeartbeatFactory.create(runtimeInfoWrapper);
- client.publishHeartbeat(c2Heartbeat).ifPresent(this::processResponse);
+ C2Heartbeat c2Heartbeat =
c2HeartbeatFactory.create(runtimeInfoWrapper);
Review Comment:
This indentation seems unnecessary could you please double check?
##########
c2/c2-client-bundle/c2-client-service/src/main/java/org/apache/nifi/c2/client/service/operation/UpdateConfigurationOperationHandler.java:
##########
@@ -75,30 +75,44 @@ public C2OperationAck handle(C2Operation operation) {
.map(map -> map.get(LOCATION))
.orElse(EMPTY);
- String newFlowId = parseFlowId(updateLocation);
- if (flowIdHolder.getFlowId() == null ||
!flowIdHolder.getFlowId().equals(newFlowId)) {
- logger.info("Will perform flow update from {} for operation #{}.
Previous flow id was {}, replacing with new id {}", updateLocation,
opIdentifier,
- flowIdHolder.getFlowId() == null ? "not set" :
flowIdHolder.getFlowId(), newFlowId);
+ String flowId = getFlowId(operation.getArgs(), updateLocation);
+ if (flowId == null) {
+ state.setState(C2OperationState.OperationState.NOT_APPLIED);
+ state.setDetails("Could not get flowId from the operation.");
+ logger.info("FlowId is missing, no update will be performed.");
} else {
- logger.info("Flow is current, no update is necessary...");
+ if (flowIdHolder.getFlowId() == null ||
!flowIdHolder.getFlowId().equals(flowId)) {
+ logger.info("Will perform flow update from {} for operation
#{}. Previous flow id was {}, replacing with new id {}", updateLocation,
opIdentifier,
+ flowIdHolder.getFlowId() == null ? "not set" :
flowIdHolder.getFlowId(), flowId);
+ } else {
+ logger.info("Flow is current, no update is necessary...");
+ }
+ flowIdHolder.setFlowId(flowId);
+ state.setState(updateFlow(opIdentifier, updateLocation));
}
+ return operationAck;
+ }
- flowIdHolder.setFlowId(newFlowId);
+ private C2OperationState.OperationState updateFlow(String opIdentifier,
String updateLocation) {
Optional<byte[]> updateContent =
client.retrieveUpdateContent(updateLocation);
if (updateContent.isPresent()) {
if (updateFlow.apply(updateContent.get())) {
- state.setState(C2OperationState.OperationState.FULLY_APPLIED);
logger.debug("Update configuration applied for operation
#{}.", opIdentifier);
+ return C2OperationState.OperationState.FULLY_APPLIED;
} else {
- state.setState(C2OperationState.OperationState.NOT_APPLIED);
logger.error("Update resulted in error for operation #{}.",
opIdentifier);
+ return C2OperationState.OperationState.NOT_APPLIED;
}
} else {
- state.setState(C2OperationState.OperationState.NOT_APPLIED);
logger.error("Update content retrieval resulted in empty content
so flow update was omitted for operation #{}.", opIdentifier);
+ return C2OperationState.OperationState.NOT_APPLIED;
}
+ }
- return operationAck;
+ private String getFlowId(Map<String, String> args, String updateLocation) {
+ Optional<String> flowId = Optional.ofNullable(args)
Review Comment:
You don't need this local varibale you could simply chain further
```
return Optional.ofNullable(args)
.map(map -> map.get(FLOW_ID))
.orElseGet(() -> parseFlowId(updateLocation));
```
What do you think?
##########
minifi/minifi-integration-tests/src/test/resources/c2/protocol/minifi-edge3/expected.json:
##########
@@ -0,0 +1,17 @@
+[
+ {
+ "pattern": "200 OK https://c2-authoritative:10443/c2/config/heartbeat";
+ },
+ {
+ "pattern": "\"operation\":\"UPDATE\",\"operand\":\"CONFIGURATION\""
+ },
+ {
+ "pattern": "200 OK https://c2-authoritative:10443/c2/config\\?class=raspi4";
+ },
+ {
+ "pattern": "200 OK https://c2-authoritative:10443/c2/config/acknowledge";
+ },
+ {
+ "pattern": "^__testTextRaspi4__$"
Review Comment:
Thanks for this, I know it wasn't straightforward but this is really
important that now we are validating the parallel handling / publishing of
different agent 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
