You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2021/10/01 08:23:53 UTC

[GitHub] [nifi] adenes commented on a change in pull request #5402: NIFI-9229 Flow upgrade not possible if a Output Port changes to a funnel

adenes commented on a change in pull request #5402:
URL: https://github.com/apache/nifi/pull/5402#discussion_r720039865



##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/groups/StandardProcessGroup.java
##########
@@ -4293,16 +4294,30 @@ private void updateProcessGroup(final ProcessGroup group, final VersionedProcess
         //As Input Port (IP1) originally belonged to PGA the new connection would be incorrectly linked to the old Input Port
         //instead of the one being in PGB, so it needs to be removed first before updating the connections.
 
-        for (final String removedVersionedId : inputPortsRemoved) {
+        Iterator<String> inputPortsRemovedIterator = inputPortsRemoved.iterator();
+        while (inputPortsRemovedIterator.hasNext()) {
+            final String removedVersionedId = inputPortsRemovedIterator.next();
             final Port port = inputPortsByVersionedId.get(removedVersionedId);
             LOG.info("Removing {} from {}", port, group);
-            group.removeInputPort(port);
+            try {
+                group.removeInputPort(port);
+                inputPortsRemovedIterator.remove();
+            } catch (IllegalStateException e) {
+                LOG.info("Removing {} from {} not possible at the moment, connection needs to be updated first.", port, group);

Review comment:
       Even though this is an info log message at first sight my impression was that I as a user need to "update the connections". Please update this to something like "...not possible at the moment, will try again after updating the connections."

##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/groups/StandardProcessGroup.java
##########
@@ -4293,16 +4294,30 @@ private void updateProcessGroup(final ProcessGroup group, final VersionedProcess
         //As Input Port (IP1) originally belonged to PGA the new connection would be incorrectly linked to the old Input Port
         //instead of the one being in PGB, so it needs to be removed first before updating the connections.
 
-        for (final String removedVersionedId : inputPortsRemoved) {
+        Iterator<String> inputPortsRemovedIterator = inputPortsRemoved.iterator();
+        while (inputPortsRemovedIterator.hasNext()) {
+            final String removedVersionedId = inputPortsRemovedIterator.next();
             final Port port = inputPortsByVersionedId.get(removedVersionedId);
             LOG.info("Removing {} from {}", port, group);
-            group.removeInputPort(port);
+            try {
+                group.removeInputPort(port);
+                inputPortsRemovedIterator.remove();
+            } catch (IllegalStateException e) {
+                LOG.info("Removing {} from {} not possible at the moment, connection needs to be updated first.", port, group);
+            }
         }
 
-        for (final String removedVersionedId : outputPortsRemoved) {
+        Iterator<String> outputPortsRemovedIterator = outputPortsRemoved.iterator();
+        while (outputPortsRemovedIterator.hasNext()) {
+            final String removedVersionedId = outputPortsRemovedIterator.next();
             final Port port = outputPortsByVersionedId.get(removedVersionedId);
             LOG.info("Removing {} from {}", port, group);
-            group.removeOutputPort(port);
+            try {
+                group.removeOutputPort(port);
+                outputPortsRemovedIterator.remove();
+            } catch (IllegalStateException e) {
+                LOG.info("Removing {} from {} not possible at the moment, connection needs to be updated first.", port, group);

Review comment:
       same as above

##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/groups/StandardProcessGroup.java
##########
@@ -4343,6 +4358,18 @@ private void updateProcessGroup(final ProcessGroup group, final VersionedProcess
             group.removeFunnel(funnel);
         }
 
+        for (final String removedVersionedId : inputPortsRemoved) {

Review comment:
       No strong opinion on this, but maybe a log message or a comment would be useful here: "removing the remaining input and output ports". 

##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/groups/StandardProcessGroup.java
##########
@@ -4730,7 +4757,8 @@ private Connectable getConnectable(final ProcessGroup group, final ConnectableCo
                 // if the flow doesn't contain the properly mapped group id, we need to search all child groups.
                 return group.getProcessGroups().stream()
                     .flatMap(gr -> gr.getInputPorts().stream())
-                    .filter(component -> id.equals(component.getVersionedComponentId().orElse(
+                    .filter(component

Review comment:
       minor: unnecessary formatting change, please revert




-- 
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