kevdoran commented on issue #230: NIFIREG-321 Integrate revision concept into UI, REST API, and service… URL: https://github.com/apache/nifi-registry/pull/230#issuecomment-539646724 @bbende Thanks for this contribution. This will be an important addition for HA deployment scenarios. I have taken a quick look at the code changes. I have not built or run this branch yet, but I wanted to provide you an update and initial feedback. I have reservations about adding the clientId field to the `RevisionInfo` object for NiFi Registry DTOs. I am aware that this is is being brought over from NiFi and that this design is consistent with NiFi. I am not aware of what value it adds to NiFi Registry (will our frontend or the nifi client use it?), and I think it comes with potential downsides: 1. If the plan for `RevisionInfo` is (at least partially) to allow for optimistic locking and prevent lost/overwritten updates in a concurrent access situation, then misuse (intentional or accidental) of the clientId field allows for overriding another client's update. 2. It adds complexity and confusion to the REST API documentation and correct usage for newcomers. A recent example of this from the nifi side: https://apachenifi.slack.com/archives/C0L9VCD47/p1570538029297700 If the plan is that the new library modules introduced by this PR could eventually move to `nifi-standard-libraries` to be shared across `nifi` and `nifi-registry`, then I am in support of having an _optional_ clientId field that the library understands, so that it can be used in the nifi case. However, I don't think NiFi Registry should be taking advantage of it in its DTOs.
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
