simonbence commented on code in PR #8726:
URL: https://github.com/apache/nifi/pull/8726#discussion_r1590716881
##########
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:
I have no strong feelings about it but I see some possible benefit:
- 1. It looks generally a good approach to not fully rely on client side
data validation (due to possible changes there, etc.)
- 2I did see some usage where NiFi API was used directly from
scripts/automazation. While I do not think that this is the intended way
(however by supporting "Request Auth Token" this seems inevidable) there is a
possibility for this and I would be on the safe side even if the current
version of the code could not done real harm with using non-existing branch.
I am not sure which is the more forwarding way to tackle this. We can either
add validation (it looks a bit much future proof but might be misleading as it
would look like this is an argument we really consider here) or just enforce
the default value (which comes with some hardwired thing, generally to avoid)
but I think it would be safer to do some steps towards some of these directions.
--
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]
