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/05/23 16:41:04 UTC
[GitHub] [nifi] markobean opened a new pull request #5094: NIFI-8195: add default connection settings to process group configura…
markobean opened a new pull request #5094:
URL: https://github.com/apache/nifi/pull/5094
…tion
<!--
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:
#### 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:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [x] 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.
- [x] Has your PR been rebased against the latest commit within the target branch (typically `main`)?
- [x] 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:
- [x] Have you ensured that the full suite of tests is executed via `mvn -Pcontrib-check clean install` at the root `nifi` folder?
- [x] Have you written or updated unit tests to verify your changes?
- [x] Have you verified that the full build is successful on JDK 8?
- [x] 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:
- [x] 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.
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
Verified the 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
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 commented on pull request #5094: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
markap14 commented on pull request #5094:
URL: https://github.com/apache/nifi/pull/5094#issuecomment-867878823
Thanks for the contribution and thanks for spending the time to review @mattyb149 . But I think this PR is problematic.
The Jira spells this out as adding a default backpressure at the Process Group level. I don't think this is necessary, but I don't necessarily have a problem with it. However, the PR doesn't appear to update VersionedProcessGroup, which means that after it's been pushed to the registry and then pulled out again, the newly created one will not function like the old one did - it won't have the default settings. That needs to be addressed.
However, more concerning is the introduction of the default FlowFile Expiration. We should not be introducing FlowFile Expiration without the user explicitly setting it. This was discussed at length previously in https://github.com/apache/nifi/pull/4800
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 commented on pull request #5094: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
mattyb149 commented on pull request #5094:
URL: https://github.com/apache/nifi/pull/5094#issuecomment-867846166
+1 LGTM, tried various PGs, downloaded the flow definition and ensured all connections looked as expected. Thanks for this 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 commented on pull request #5094: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
mattyb149 commented on pull request #5094:
URL: https://github.com/apache/nifi/pull/5094#issuecomment-867727381
Reviewing...
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 closed pull request #5094: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
mattyb149 closed pull request #5094:
URL: https://github.com/apache/nifi/pull/5094
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 edited a comment on pull request #5094: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
mattyb149 edited a comment on pull request #5094:
URL: https://github.com/apache/nifi/pull/5094#issuecomment-867915139
I also saw some warnings about the unrecognized new fields:
```
2021-06-24 14:13:10,238 WARN [main] org.apache.nifi.util.FlowParser
Schema validation error parsing Flow Configuration at line 26, col 32:
cvc-complex-type.2.4.a: Invalid content was found starting with
element 'defaultFlowFileExpiration'. One of '{processor, inputPort,
outputPort, label, funnel, processGroup, remoteProcessGroup,
connection, controllerService, template, variable,
parameterContextId}' is expected.
2021-06-24 14:13:10,369 WARN [main] org.apache.nifi.util.FlowParser
Schema validation error parsing Flow Configuration at line 1195, col
34: cvc-complex-type.2.4.a: Invalid content was found starting with
element 'defaultFlowFileExpiration'. One of
'{versionControlInformation, processor, inputPort, outputPort, label,
funnel, processGroup, remoteProcessGroup, connection,
controllerService, template, variable, parameterContextId}' is
expected.
2021-06-24 14:13:10,433 WARN [main] org.apache.nifi.util.FlowParser
Schema validation error parsing Flow Configuration at line 1915, col
34: cvc-complex-type.2.4.a: Invalid content was found starting with
element 'defaultFlowFileExpiration'. One of
'{versionControlInformation, processor, inputPort, outputPort, label,
funnel, processGroup, remoteProcessGroup, connection,
controllerService, template, variable, parameterContextId}' is
expected.
```
At this point I think the PR needs to be reverted, we can continue discussions on Github and/or Jira, but it seems the PR wasn't ready as-is.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 commented on pull request #5094: NIFI-8195: add default connection settings to process group configura…
Posted by GitBox <gi...@apache.org>.
mattyb149 commented on pull request #5094:
URL: https://github.com/apache/nifi/pull/5094#issuecomment-867915139
I also saw some warnings about the unrecognized new fields:
```2021-06-24 14:13:10,238 WARN [main] org.apache.nifi.util.FlowParser
Schema validation error parsing Flow Configuration at line 26, col 32:
cvc-complex-type.2.4.a: Invalid content was found starting with
element 'defaultFlowFileExpiration'. One of '{processor, inputPort,
outputPort, label, funnel, processGroup, remoteProcessGroup,
connection, controllerService, template, variable,
parameterContextId}' is expected.
2021-06-24 14:13:10,369 WARN [main] org.apache.nifi.util.FlowParser
Schema validation error parsing Flow Configuration at line 1195, col
34: cvc-complex-type.2.4.a: Invalid content was found starting with
element 'defaultFlowFileExpiration'. One of
'{versionControlInformation, processor, inputPort, outputPort, label,
funnel, processGroup, remoteProcessGroup, connection,
controllerService, template, variable, parameterContextId}' is
expected.
2021-06-24 14:13:10,433 WARN [main] org.apache.nifi.util.FlowParser
Schema validation error parsing Flow Configuration at line 1915, col
34: cvc-complex-type.2.4.a: Invalid content was found starting with
element 'defaultFlowFileExpiration'. One of
'{versionControlInformation, processor, inputPort, outputPort, label,
funnel, processGroup, remoteProcessGroup, connection,
controllerService, template, variable, parameterContextId}' is
expected.```
At this point I think the PR needs to be reverted, we can continue discussions on Github and/or Jira, but it seems the PR wasn't ready as-is.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org