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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
