bbende commented on code in PR #8726: URL: https://github.com/apache/nifi/pull/8726#discussion_r1589333765
########## nifi-extension-bundles/nifi-flow-registry-client-bundle/nifi-flow-registry-client-services/src/main/java/org/apache/nifi/registry/flow/NifiRegistryFlowRegistryClient.java: ########## @@ -149,7 +149,7 @@ public boolean isStorageLocationApplicable(final FlowRegistryClientConfiguration } @Override - public Set<FlowRegistryBucket> getBuckets(final FlowRegistryClientConfigurationContext context) throws FlowRegistryException, IOException { + public Set<FlowRegistryBucket> getBuckets(final FlowRegistryClientConfigurationContext context, final String branch) throws FlowRegistryException, IOException { Review Comment: Are you saying that you think we should check the `branch` argument in all of the methods in `NiFiRegistryFlowRegistryClient` to validate that it matches the default branch? The only way this could happen is someone making API calls outside of the UI where they could submit a request for save/import with a branch name that wasn't the default branch NiFi Registry is expecting, but also it would just be ignored by all of the methods, so it wouldn't cause any issues. One other note... I did not add `branch` to the NiFi Registry specific data model class like `VersionedFlow`, `VersionedFlowSnapshot`, and `VersionedFlowSnapshotMetadat`, because its not used for anything on registry side. So in your example, if someone calls `getFlowContents` with `branch` as `dev`, they will get back a null branch in the response since there wasn't a branch in the content returned from registry. Happy to improve something here, just want to make sure I'm following what you are suggesting. -- 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