You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2021/06/26 14:17:02 UTC
[GitHub] [nifi] markobean opened a new pull request #5192: NIFI-8195: add default connection settings to process group configura…
markobean opened a new pull request #5192:
URL: https://github.com/apache/nifi/pull/5192
…tion
- include new process group property support in NiFi Registry
- updated documentation to describe and show new feature
- added elements to XSD schema definition
<!--
Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements. See the NOTICE file distributed with
this work for additional information regarding copyright ownership.
The ASF licenses this file to You under the Apache License, Version 2.0
(the "License"); you may not use this file except in compliance with
the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
Thank you for submitting a contribution to Apache NiFi.
Please provide a short description of the PR here:
Adds properties to the Process Group configuration setting the default values for all newly created connections. This facilitates easier configuration during flow creation especially with multi-tenancy in mind. There are 3 properties added: default flowfile expiration, default back pressure object threshold, default back pressure data size threshold.
See updated User Guide for additional information.
The new behavior includes a basic unit test. It was also tested extensively in a running NiFi as some features are heavily UI-dependent.
Verified the new elements of the Process Group (PG) DTO are populated into flow.xml.gz when NiFi is started using an older format, i.e. without the new elements included; no warnings are issued to nifi-app.log
Verified export/import of process groups maintains property settings when using templates, NiFi Registry and JSON flow definition
Verified the default values of the added elements are “0 sec” for FlowFile Expiration, and obtained from nifi.properties for back pressure settings
Verified PG settings are displayed as non-editable when user is not in the ‘modify the component’ policy.
Verified PG settings are displayed as “Unauthorized” when user is not in the ‘view the component’ policy.
Verified new connection defaults change as the PG settings change
Verified the settings of existing connections do not change as the PG settings change
Verified newly created PG inherits strings from parent
Verified settings of existing child PG do not change as parent PG settings change
Verified User Guide documentation updated and displays correctly
#### 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 `squash` or use `--force` when pushing to allow for clean monitoring of changes._
### For code changes:
- [ ] Have you ensured that the full suite of tests is executed via `mvn -Pcontrib-check clean install` at the root `nifi` folder?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] Have you verified that the full build is successful on JDK 8?
- [ ] Have you verified that the full build is successful on JDK 11?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the `LICENSE` file, including the main `LICENSE` file under `nifi-assembly`?
- [ ] If applicable, have you updated the `NOTICE` file, including the main `NOTICE` file found under `nifi-assembly`?
- [ ] If adding new Properties, have you added `.displayName` in addition to .name (programmatic access) for each of the new properties?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### 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.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markobean commented on pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
markobean commented on pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#issuecomment-874937062
@mattyb149 Just wanted to clarify if your question was based on observed behavior (which I can't reproduce), or if it was a general question based on how you believed it would work?
Also - hoping this PR can get across the finish line before the 1.14.0 RC is initiated.
Thanks for your time and effort!
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markobean commented on a change in pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
markobean commented on a change in pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#discussion_r665553747
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/StandardFlowSynchronizer.java
##########
@@ -1197,9 +1197,15 @@ private void updateProcessGroup(final ProcessGroup group, final ProcessGroupDTO
}
}
- group.setDefaultFlowFileExpiration(defaultFlowFileExpiration);
- group.setDefaultBackPressureObjectThreshold(defaultBackPressureObjectThreshold);
- group.setDefaultBackPressureDataSizeThreshold(defaultBackPressureDataSizeThreshold);
+ if (defaultFlowFileExpiration == null) {
Review comment:
Yes, they should. I was moving too quickly and using the above flowFileOutboundPolicyName as an example.. but that is just setting defaults in case of null.
Fixed.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markobean commented on pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
markobean commented on pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#issuecomment-875772050
Rebased to main and running contrib-check now
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markobean commented on pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
markobean commented on pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#issuecomment-875130866
Yeah, that WIP commit was simply a placeholder after I took 5 minutes to look at it from work, and commit so I could continue after a fresh pull at home. Just reading your comment now. Will fix this up ASAP. Stay tuned..
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 commented on a change in pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
mattyb149 commented on a change in pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#discussion_r665523695
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/StandardFlowSynchronizer.java
##########
@@ -1197,9 +1197,15 @@ private void updateProcessGroup(final ProcessGroup group, final ProcessGroupDTO
}
}
- group.setDefaultFlowFileExpiration(defaultFlowFileExpiration);
- group.setDefaultBackPressureObjectThreshold(defaultBackPressureObjectThreshold);
- group.setDefaultBackPressureDataSizeThreshold(defaultBackPressureDataSizeThreshold);
+ if (defaultFlowFileExpiration == null) {
Review comment:
Shouldn't these be `!= null` ? We only want to set the values if they're not null, they'll be null in the REST call when you move the component
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 commented on pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
mattyb149 commented on pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#issuecomment-875066826
I was able to reproduce with the following:
1) Starting from a blank canvas/flow, create a new PG at the root level called 'test1'
2) Right-click on the PG and change the values for default backpressure to 20000 and 2 GB. Click Apply
3) Go into the PG and create a new PG called 'child1'
4) Right-click and configure the child1 PG, verify the values were inherited from the test1 PG. Click Cancel
5) Move the child PG on the canvas
6) Right-click and configure the child1 PG, verify the values are the original defaults (10000 and 1 GB)
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 commented on pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
mattyb149 commented on pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#issuecomment-875101003
Not sure if your latest WIP commit is the right approach, check my latest comment on StandardProcessGroupDAO
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 closed pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
mattyb149 closed pull request #5192:
URL: https://github.com/apache/nifi/pull/5192
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markobean commented on pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
markobean commented on pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#issuecomment-870773704
When creating a new process group, the properties will inherit from its immediate parent. The backpressure defaults from nifi.properties should only be used when at the root level, and the root has not specified a value (e.g. flow.xml.gz from an older NiFi where these properties have not been specified.)
After a process groups is created, the properties are set. These values - and not any type of inheritance - will be used when creating a new connection.
Where do you see inheritance from nifi.properties vice immediate parent?
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markobean commented on pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
markobean commented on pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#issuecomment-875631065
Clarifying the outcome of the latest push. I followed your steps to reproduce, and the new PG properties maintained their values, as expected.
1. Starting from a blank canvas/flow, create a new PG at the root level called 'test1'
2. Right-click on the PG and change the values for default backpressure to 20000 and 2 GB. Click Apply
3. Go into the PG and create a new PG called 'child1'
4. Right-click and configure the child1 PG, verify the values were inherited from the test1 PG. Click Cancel
5. Move the child PG on the canvas
6. Right-click and configure the child1 PG, verify the values **are still 20000 and 2GB**
The above verified custom default FlowFile expiration setting as well. Also, I tried copy/paste; it maintained values as expected.
The Java 8 contrib-check passed, but I'm getting the following error for Java 11 contrib-check. Maybe it is strictly an environmental thing with my VM. Have you seen this before?
`[ERROR] Failed to execute goal on project minifi-c2-integration-tests: Could not resolve dependencies for project org.apache.nifi.minifi:minifi-c2-integration-tests:jar:1.14.0-SNAPSHOT: Failed to collect dependencies at com.palantir.docker.compose:docker-compose-rule-junit4:jar:0.33.0: Failed to read artifact descriptor for com.palantir.docker.compose:docker-compose-rule-junit4:jar:0.33.0: Could not transfer artifact com.palantir.docker.compose:docker-compose-rule-junit4:pom:0.33.0 from/to palantir.bintray (https://dl.bintray.com/palantir/releases): authorization failed for https://dl.bintray.com/palantir/releases/com/palantir/docker/compose/docker-compose-rule-junit4/0.33.0/docker-compose-rule-junit4-0.33.0.pom, status: 403 Forbidden -> [Help 1]`
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 commented on pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
mattyb149 commented on pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#issuecomment-875956406
+1 LGTM, ran a few different scenarios and verified the results were as expected. Thanks for the feature! Merging to main
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markobean commented on pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
markobean commented on pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#issuecomment-875036011
Awesome. Thanks @mattyb149
And, if you find a case where properties are inherited from nifi.properties (or anywhere other than the PG's immediate parent), please let me know.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markobean commented on pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
markobean commented on pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#issuecomment-875847911
Java 8 contrib-check passed as well. Good to go unless you have any additional comments @mattyb149
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 commented on pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
mattyb149 commented on pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#issuecomment-870759144
Should a child process group inherit from its immediate parent? Looks like any PG will be created with the nifi.properties defaults rather than its immediate parent.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 commented on a change in pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
mattyb149 commented on a change in pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#discussion_r664895968
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/dao/impl/StandardProcessGroupDAO.java
##########
@@ -373,6 +373,11 @@ public ProcessGroup updateProcessGroup(ProcessGroupDTO processGroupDTO) {
if (flowFileOutboundPolicy != null) {
group.setFlowFileOutboundPolicy(flowFileOutboundPolicy);
}
+
+ group.setDefaultFlowFileExpiration(processGroupDTO.getDefaultFlowFileExpiration());
Review comment:
When the position is changed, all other fields in the DTO are null. When `getDefaultFlowFileExpiration()` is null, `setDefaultFlowFileExpiration()` sets the system default. These method calls should check for null like the code right above it, and not call `set` if the value is null (but leave the defensive code in StandardProcessGroup)
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 commented on pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
mattyb149 commented on pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#issuecomment-875028755
I'm doing a final build/review/test before merging, hopefully merged in an hour or so
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 commented on pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
mattyb149 commented on pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#issuecomment-875739763
Did you rebase your branch against main? That error was fixed via NIFI-8757 (#5198)
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markobean commented on pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
markobean commented on pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#issuecomment-875816755
Java 11 contrib-check succeeded, so I pushed update. Running Java 8 contrib-check now.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markobean commented on pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
markobean commented on pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#issuecomment-875080985
Ok, good catch. I guess I never moved the PG. That's odd behavior to say the least. I don't know why simply moving the PG would cause it to revert. I'll look into it.
Any ideas off the top of your head why a placement change would cause properties to change?
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markobean commented on pull request #5192: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
markobean commented on pull request #5192:
URL: https://github.com/apache/nifi/pull/5192#issuecomment-875188312
@mattyb149 I force pushed an update. (The force was to overwrite the "wip" which was not necessary.) I also discovered a related issue in StandardFlowSynchronizer.java which I believe requires a similar NULL check for these properties.
I pushed quickly as I don't expect these changes to violate the contrib-check profile, but I'm running that now just to be sure.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org