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/04/06 12:58:26 UTC

[GitHub] [nifi] tpalfy commented on a change in pull request #4516: NIFI-7788 Start and Stop actions on a Process Group should enable/disable transmission of RPG's

tpalfy commented on a change in pull request #4516:
URL: https://github.com/apache/nifi/pull/4516#discussion_r604095650



##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/RemoteProcessGroupResource.java
##########
@@ -829,6 +833,105 @@ public Response updateRemoteProcessGroupRunStatus(
         );
     }
 
+    /**
+     * Updates the operational status for all remote process groups in the specified process group with the specified value.
+     *
+     * @param httpServletRequest                request
+     * @param processGroupId                    The id of the process group in which all remote process groups to update.
+     * @param requestRemotePortRunStatusEntity  A remotePortRunStatusEntity that holds the desired run status
+     * @return A response with an array of RemoteProcessGroupEntity objects.
+     */
+    @PUT
+    @Consumes(MediaType.APPLICATION_JSON)
+    @Produces(MediaType.APPLICATION_JSON)
+    @Path("process-group/run-status/{id}")

Review comment:
       I saw no good pattern for this so I came up with this one: I added `/process-group` to indicate that the scope of the operation is all RPGs within a process group.
   
   As for the other part, nowhere else do we use `run-statuses` in this plural form in the urls.
   Method names should be more precise because they also serve as a form of documentation. If the method name was `updateRemoteProcessGroupRunStatus` it would imply that it updates the run status of a single RPG.
   
   URLs on the other hand don't need to have this implication. They merely indicate the type of the resources they manage. For example plurality can be even dependent on the payload instead of the URL.
   
   Overall I'd rather leave it as it is.




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

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