NIFI-9038: Fix fingerprinting group access control policies for Remote Port#5300
NIFI-9038: Fix fingerprinting group access control policies for Remote Port#5300naddym wants to merge 1 commit intoapache:mainfrom
Conversation
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for addressing this issue @naddym! What do you think about adjusting some of the sample flow definitions to test this change?
|
Thank you @exceptionfactory for the review. I have made requested changes. Please let me know for anything further. Thanks again. |
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for adding the unit tests @naddym! This looks close to completion, just wondering about the addition of flowfileConcurrency and flowfileOutboundPolicy elements in multiple test files. Were those changes necessary for the tests to pass?
| <flowfileConcurrency>UNBOUNDED</flowfileConcurrency> | ||
| <flowfileOutboundPolicy>STREAM_WHEN_AVAILABLE</flowfileOutboundPolicy> |
There was a problem hiding this comment.
Are these additional elements required for this change?
There was a problem hiding this comment.
Thanks again @exceptionfactory for the review. No, those changes are not required. During development of flowfile concurrency feature for a process group, the flow configuration files weren't updated. I thought of adding those since I'm anyway modifying those files. Just updating the files to what it looks like today in flow.xml.gz as it also gets fingerprinted. I can revert back the change if it is not relevant to this PR. Please let me know
There was a problem hiding this comment.
Thanks for the confirmation @naddym, that's helpful to know. Leaving the changes seems acceptable, I just wanted to confirm that they weren't necessary for the fingerprint calculation.
Since the primary change is correcting the code to evaluate groupAccessControl properly, wanted to make sure I wasn't missing something.
There was a problem hiding this comment.
Thanks @exceptionfactory. That was a valid question. I should have been more specific while describing the PR on additional changes. Will take care of it from next time. Thanks again.
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks again @naddym, looks good! +1 Merging.
Port This closes apache#5300 Signed-off-by: David Handermann <exceptionfactory@apache.org>
Thank you for submitting a contribution to Apache NiFi.
Please provide a short description of the PR here:
Description of PR
Enables X functionality; fixes bug NIFI-YYYY.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically
main)?Is your initial contribution a single, squashed commit? Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not
squashor use--forcewhen pushing to allow for clean monitoring of changes.For code changes:
mvn -Pcontrib-check clean installat the rootnififolder?LICENSEfile, including the mainLICENSEfile undernifi-assembly?NOTICEfile, including the mainNOTICEfile found undernifi-assembly?.displayNamein addition to .name (programmatic access) for each of the new properties?For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI for build issues and submit an update to your PR as soon as possible.