Skip to content

NIFI-10918: When fetching a flow from a Flow Registry, if it referenc…#6736

Closed
markap14 wants to merge 2 commits intoapache:mainfrom
markap14:NIFI-10918
Closed

NIFI-10918: When fetching a flow from a Flow Registry, if it referenc…#6736
markap14 wants to merge 2 commits intoapache:mainfrom
markap14:NIFI-10918

Conversation

@markap14
Copy link
Contributor

…es any 'internal versioned flows' instead of requiring that we have a client configured for the appropriate URL, attempt to fetch the flow from each client. We will start with the clients that do report that they can handle the URL but will try others as well. As soon as we successfully fetch the flow, we stop.

Summary

NIFI-00000

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Pull Request Tracking

  • Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

  • Pull Request based on current revision of the main branch
  • Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

  • Build completed using mvn clean install -P contrib-check
    • JDK 8
    • JDK 11
    • JDK 17

Licensing

  • New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • Documentation formatting appears as expected in rendered files

…es any 'internal versioned flows' instead of requiring that we have a client configured for the appropriate URL, attempt to fetch the flow from each client. We will start with the clients that do report that they can handle the URL but will try others as well. As soon as we successfully fetch the flow, we stop.
logger.debug("Successfully fetched flow for Bucket [{}] Flow [{}] Version [{}] using {}", bucketId, flowId, version, clientNode);
return snapshot;
} catch (final Exception e) {
logger.debug("Failed to fetch flow", e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would worth to provide detail about the registry and flow

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't bother with logging that because I thought it too verbose. Specifically, because it's at a debug level and about 5 lines above we log very specifically the flow that we're about to fetch. Because of that, I didn't want to overcomplicate the log message.

// Sort clients based on whether or not they believe they are applicable for the given storage location
final List<FlowRegistryClientNode> matchingClients = new ArrayList<>(flowManager.getAllFlowRegistryClients());
matchingClients.sort(Comparator.comparing(client -> client.isStorageLocationApplicable(storageLocation) ? -1 : 1));
return matchingClients;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered using the isStorageLocationApplicable as filter instead for an extra safeguard? (Like it would try not every clients but every clients returning true for that call). My reasoning: identifiers of flows and buckets are generated by the registry implementations, which might differ case by case. It is possible that the ids are for example sequentials and different flows might exist with the same bucket and flow id, but without other relation? Like in this case if the bucket id is 1 and flow id is 1, and there are multiple registry instances with an implementation like this, the first will be loaded. Which in this case not necessary a flow which "matches" the storage location, might result an unexpected flow to be loaded

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, I didn't consider the idea of buckets and flows having one-up IDs. I suppose it's possible. We could update the documentation to recommend against that.

We need to check all registry clients, however, because a URL is not sufficient to know whether or not a given client is applicable. For example, if the port or hostname of the registry changes (as in the example in the Jira, when the registry went from being non-secure to secure) all of a sudden we can no longer load any flow that has an 'inner versioned flow' simply because the URL is no longer accurate.

@simonbence
Copy link
Contributor

I execute the test case I added to the ticket. If it is successfull, I merge this into the main.

@simonbence
Copy link
Contributor

The code passed the test case successfully and also the debug shows that it tries to load the flow from the incorrect repository, then it falls back and loads it correctly. I will merge

@asfgit asfgit closed this in 2473683 Dec 2, 2022
asfgit pushed a commit that referenced this pull request Dec 3, 2022
…es any 'internal versioned flows' instead of requiring that we have a client configured for the appropriate URL, attempt to fetch the flow from each client. We will start with the clients that do report that they can handle the URL but will try others as well. As soon as we successfully fetch the flow, we stop.

NIFI-10918: Fixed checkstyle violations

This closes #6736
Signed-off-by: Bence Simon <bsimon@apache.org>
lizhizhou pushed a commit to lizhizhou/nifi that referenced this pull request Jan 2, 2023
…es any 'internal versioned flows' instead of requiring that we have a client configured for the appropriate URL, attempt to fetch the flow from each client. We will start with the clients that do report that they can handle the URL but will try others as well. As soon as we successfully fetch the flow, we stop.

NIFI-10918: Fixed checkstyle violations

This closes apache#6736
Signed-off-by: Bence Simon <bsimon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants