[jira] [Created] (NIFI-7596) InvokeHTTP - Get Request Body From Attribute
Firenz created NIFI-7596: Summary: InvokeHTTP - Get Request Body From Attribute Key: NIFI-7596 URL: https://issues.apache.org/jira/browse/NIFI-7596 Project: Apache NiFi Issue Type: Improvement Components: Extensions Reporter: Firenz *Use case :* the flow file contains 1 big data (ex : PDF or pictures), but you want to lookup metadata with InvokeHTTP using a POST (or PUT). POST is often used in http API for "search" with lot of params or list of params. *Proposition* : In the InvokeHTTP, add a "Get Request Body From Attribute" (like Put Response Body In Attribute). If this field not empty, the chosen attribute will be read as the request, insted of the flow file. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] alopresto commented on a change in pull request #4377: NIFI-7568 - Fixing Kerberos mappings
alopresto commented on a change in pull request #4377: URL: https://github.com/apache/nifi/pull/4377#discussion_r448674389 ## File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-administration/src/main/java/org/apache/nifi/admin/service/impl/StandardKeyService.java ## @@ -119,10 +119,18 @@ public void deleteKey(String identity) { // delete the keys DeleteKeysAction deleteKeys = new DeleteKeysAction(identity); -transaction.execute(deleteKeys); - -// commit the transaction -transaction.commit(); +Integer rowsDeleted = transaction.execute(deleteKeys); + +// commit the transaction if we found one and only one matching user identity +if(rowsDeleted == 0) { +rollback(transaction); +throw new AdministrationException("Unable to find user identity " + identity + " to remove token."); +} else if(rowsDeleted > 1) { Review comment: Is there a legitimate scenario where there should be multiple identical identities in the database? If someone can take advantage of the identity mappings and cause their identity to match someone else's, this could be an attack vector. 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: us...@infra.apache.org
[GitHub] [nifi] alopresto commented on pull request #4382: NIFI-7595 Upgrade log4j-core dependency.
alopresto commented on pull request #4382: URL: https://github.com/apache/nifi/pull/4382#issuecomment-652695875 I'm going to let the other CI/CD builds finish, but I have no idea why the failing Groovy test occurred on the MacOS build. That test has been running stably for some time. 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: us...@infra.apache.org
[jira] [Updated] (NIFI-7595) Upgrade log4j-core dependency
[ https://issues.apache.org/jira/browse/NIFI-7595?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] M Tien updated NIFI-7595: - Status: Patch Available (was: Open) > Upgrade log4j-core dependency > - > > Key: NIFI-7595 > URL: https://issues.apache.org/jira/browse/NIFI-7595 > Project: Apache NiFi > Issue Type: Improvement > Components: Core Framework >Affects Versions: 1.11.4 >Reporter: M Tien >Assignee: M Tien >Priority: Major > Labels: dependencies > Time Spent: 10m > Remaining Estimate: 0h > > Upgrade org.apache.logging.log4j:log4j-core dependency. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] mtien-apache opened a new pull request #4382: NIFI-7595 Upgrade log4j-core dependency.
mtien-apache opened a new pull request #4382: URL: https://github.com/apache/nifi/pull/4382 Thank you for submitting a contribution to Apache NiFi. Please provide a short description of the PR here: Description of PR _Upgrade log4j-core dependency to 2.13.3._ 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-** where 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 `master`)? - [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? - [ ] Have you written or updated unit tests to verify your changes? - [x] 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] arpadboda commented on pull request #829: WIP: MINIFICPP-1279 - FlowFile shouldn't inherit from Connectable
arpadboda commented on pull request #829: URL: https://github.com/apache/nifi-minifi-cpp/pull/829#issuecomment-652651449 This is tagged for 1.0.0 and WIP, please do NOT merge under any circumstance! 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: us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] arpadboda opened a new pull request #829: WIP: MINIFICPP-1279 - FlowFile shouldn't inherit from Connectable
arpadboda opened a new pull request #829: URL: https://github.com/apache/nifi-minifi-cpp/pull/829 Thank you for submitting a contribution to Apache NiFi - MiNiFi C++. 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 MINIFICPP- where 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 master)? - [ ] Is your initial contribution a single, squashed commit? ### For code changes: - [ ] 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? - [ ] If applicable, have you updated the NOTICE file? ### 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 travis-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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Created] (MINIFICPP-1279) FlowFile shouldn't inherit from Connectable
Arpad Boda created MINIFICPP-1279: - Summary: FlowFile shouldn't inherit from Connectable Key: MINIFICPP-1279 URL: https://issues.apache.org/jira/browse/MINIFICPP-1279 Project: Apache NiFi MiNiFi C++ Issue Type: Improvement Affects Versions: 0.7.0 Reporter: Arpad Boda Assignee: Arpad Boda Fix For: 1.0.0 FlowFileRecords consume quite a lot of memory (KBs/piece). In some cases (for eg. Windows events collected), the actual memory consumption is higher than the amount of data collected (and written to content repo). Not inheriting from Connectable could save a lot of members. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (NIFI-7595) Upgrade log4j-core dependency
M Tien created NIFI-7595: Summary: Upgrade log4j-core dependency Key: NIFI-7595 URL: https://issues.apache.org/jira/browse/NIFI-7595 Project: Apache NiFi Issue Type: Improvement Components: Core Framework Affects Versions: 1.11.4 Reporter: M Tien Assignee: M Tien Upgrade org.apache.logging.log4j:log4j-core dependency. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi-minifi] mattyb149 opened a new pull request #193: MINIFI-245: Expose content repository implementation configuration
mattyb149 opened a new pull request #193: URL: https://github.com/apache/nifi-minifi/pull/193 Thank you for submitting a contribution to Apache NiFi - MiNiFi. 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 MINIFI- where 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 master)? - [x] Is your initial contribution a single, squashed commit? ### 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-minifi folder? - [x] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](https://www.apache.org/legal/resolved.html#category-a)? - [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under minifi-assembly? - [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under minifi-assembly? ### 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 travis-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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Assigned] (NIFI-7584) LOG OUT button does not work when OpenID Connect is used for authentication
[ https://issues.apache.org/jira/browse/NIFI-7584?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Nathan Gough reassigned NIFI-7584: -- Assignee: Nathan Gough > LOG OUT button does not work when OpenID Connect is used for authentication > --- > > Key: NIFI-7584 > URL: https://issues.apache.org/jira/browse/NIFI-7584 > Project: Apache NiFi > Issue Type: Bug > Components: Core UI >Affects Versions: 1.11.4 > Environment: CentOS Linux 7 >Reporter: W Chang >Assignee: Nathan Gough >Priority: Critical > Labels: UI, bug, logout, oidc > > When nifi-1.11.4 is integrated with Okta OpenID Connect for authentication, > 'LOG OUT' button on the front page does not work. It does not log a user out > properly without redirecting to the Logout Redirect URL. > When the button is clicked, the following message is displayed on the browser > {code:java} > {"errorCode":"invalid_client","errorSummary":"Invalid value for 'client_id' > parameter.","errorLink":"invalid_client","errorId":"oae_YfJRUHCQe-BqYnPw6opFg","errorCauses":[]}{code} > The button makes a GET request to the following address. > [https://\{hostname}.okta.com/oauth2/v1/logout?post_logout_redirect_uri=https%3A%2F%2F\{nifi > server dns name}%3A\{port > number}%2Fnifi-api%2F..%2Fnifi|https://dev-309877.okta.com/oauth2/v1/logout?post_logout_redirect_uri=https%3A%2F%2Fplanet-dl-dev-1.mitre.org%3A9443%2Fnifi-api%2F..%2Fnifi] > According to Okta document > [https://developer.okta.com/blog/2020/03/27/spring-oidc-logout-options,] the > logout endpoint format should be as shown below: > {{[https://dev-123456.okta.com/oauth2/default/v1/logout?id_token_hint=]&post_logout_redirect_uri=[http://localhost:8080/]}} > > {{And it seems that post_logout_redirect_uri should be "https://\{nifi > server dns name}:\{port number}/nifi-api/access/oidc/logout"}} > > > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] gitfirenz closed pull request #4346: NIFI-7522 : Add Kafka 2.5 Processors set
gitfirenz closed pull request #4346: URL: https://github.com/apache/nifi/pull/4346 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: us...@infra.apache.org
[GitHub] [nifi] dependabot[bot] opened a new pull request #4381: Bump log4j-core from 2.8.2 to 2.13.2 in /nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-5-processors
dependabot[bot] opened a new pull request #4381: URL: https://github.com/apache/nifi/pull/4381 Bumps log4j-core from 2.8.2 to 2.13.2. [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=org.apache.logging.log4j:log4j-core&package-manager=maven&previous-version=2.8.2&new-version=2.13.2)](https://help.github.com/articles/configuring-automated-security-fixes) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) - `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language - `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language - `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language - `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/apache/nifi/network/alerts). 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: us...@infra.apache.org
[GitHub] [nifi] dependabot[bot] opened a new pull request #4380: Bump log4j-core from 2.8.2 to 2.13.2 in /nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors
dependabot[bot] opened a new pull request #4380: URL: https://github.com/apache/nifi/pull/4380 Bumps log4j-core from 2.8.2 to 2.13.2. [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=org.apache.logging.log4j:log4j-core&package-manager=maven&previous-version=2.8.2&new-version=2.13.2)](https://help.github.com/articles/configuring-automated-security-fixes) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) - `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language - `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language - `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language - `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/apache/nifi/network/alerts). 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: us...@infra.apache.org
[jira] [Updated] (NIFI-7513) Add walkthrough steps for setting dns configuration for cluster
[ https://issues.apache.org/jira/browse/NIFI-7513?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andy LoPresto updated NIFI-7513: Status: Patch Available (was: Open) > Add walkthrough steps for setting dns configuration for cluster > --- > > Key: NIFI-7513 > URL: https://issues.apache.org/jira/browse/NIFI-7513 > Project: Apache NiFi > Issue Type: Improvement > Components: Documentation & Website >Affects Versions: 1.11.4 >Reporter: Veda Kadam >Assignee: Veda Kadam >Priority: Minor > Labels: cluster, dns, dnsmasq, documentation > Time Spent: 0.5h > Remaining Estimate: 0h > > The current walkthrough describes with out dns configuration. I would like to > add steps for dns configuration. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (NIFI-7513) Add walkthrough steps for setting dns configuration for cluster
[ https://issues.apache.org/jira/browse/NIFI-7513?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andy LoPresto updated NIFI-7513: Fix Version/s: 1.12.0 Resolution: Fixed Status: Resolved (was: Patch Available) > Add walkthrough steps for setting dns configuration for cluster > --- > > Key: NIFI-7513 > URL: https://issues.apache.org/jira/browse/NIFI-7513 > Project: Apache NiFi > Issue Type: Improvement > Components: Documentation & Website >Affects Versions: 1.11.4 >Reporter: Veda Kadam >Assignee: Veda Kadam >Priority: Minor > Labels: cluster, dns, dnsmasq, documentation > Fix For: 1.12.0 > > Time Spent: 0.5h > Remaining Estimate: 0h > > The current walkthrough describes with out dns configuration. I would like to > add steps for dns configuration. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NIFI-7513) Add walkthrough steps for setting dns configuration for cluster
[ https://issues.apache.org/jira/browse/NIFI-7513?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17149609#comment-17149609 ] ASF subversion and git services commented on NIFI-7513: --- Commit 239a2e884c8a5c8215cf41c55122472e11dc419a in nifi's branch refs/heads/master from VKadam [ https://gitbox.apache.org/repos/asf?p=nifi.git;h=239a2e8 ] NIFI-7513 Added custom DNS resolution steps to walkthrough (#4359) > Add walkthrough steps for setting dns configuration for cluster > --- > > Key: NIFI-7513 > URL: https://issues.apache.org/jira/browse/NIFI-7513 > Project: Apache NiFi > Issue Type: Improvement > Components: Documentation & Website >Affects Versions: 1.11.4 >Reporter: Veda Kadam >Assignee: Veda Kadam >Priority: Minor > Labels: cluster, dns, dnsmasq, documentation > Time Spent: 0.5h > Remaining Estimate: 0h > > The current walkthrough describes with out dns configuration. I would like to > add steps for dns configuration. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] alopresto merged pull request #4359: NIFI-7513 added steps to walkthrough
alopresto merged pull request #4359: URL: https://github.com/apache/nifi/pull/4359 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: us...@infra.apache.org
[GitHub] [nifi] alopresto commented on pull request #4359: NIFI-7513 added steps to walkthrough
alopresto commented on pull request #4359: URL: https://github.com/apache/nifi/pull/4359#issuecomment-652562015 Thanks for contributing this documentation, it will be very helpful to users setting up DNS resolution. Ran `contrib-check` and all tests pass. +1, merging. 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: us...@infra.apache.org
[GitHub] [nifi-minifi] alopresto merged pull request #192: MINIFI-533: Add .asf.yaml for Github integrations
alopresto merged pull request #192: URL: https://github.com/apache/nifi-minifi/pull/192 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: us...@infra.apache.org
[GitHub] [nifi-minifi] alopresto commented on pull request #192: MINIFI-533: Add .asf.yaml for Github integrations
alopresto commented on pull request #192: URL: https://github.com/apache/nifi-minifi/pull/192#issuecomment-652549805 Thanks for fixing this Matt. 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: us...@infra.apache.org
[GitHub] [nifi-minifi] mattyb149 opened a new pull request #192: MINIFI-533: Add .asf.yaml for Github integrations
mattyb149 opened a new pull request #192: URL: https://github.com/apache/nifi-minifi/pull/192 Thank you for submitting a contribution to Apache NiFi - MiNiFi. 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 MINIFI- where 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 master)? - [x] Is your initial contribution a single, squashed commit? ### For code changes: - [ ] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi-minifi folder? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](https://www.apache.org/legal/resolved.html#category-a)? - [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under minifi-assembly? - [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under minifi-assembly? ### 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 travis-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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi] mattyb149 commented on a change in pull request #4374: NIFI-7572: Added ScriptedTransformRecord processor. Addressed a coupl…
mattyb149 commented on a change in pull request #4374: URL: https://github.com/apache/nifi/pull/4374#discussion_r448497317 ## File path: nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/ScriptedTransformRecord.java ## @@ -0,0 +1,391 @@ +/* + * 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. + */ + +package org.apache.nifi.processors.script; + +import org.apache.commons.io.IOUtils; +import org.apache.nifi.annotation.behavior.EventDriven; +import org.apache.nifi.annotation.behavior.InputRequirement; +import org.apache.nifi.annotation.behavior.SideEffectFree; +import org.apache.nifi.annotation.behavior.SupportsBatching; +import org.apache.nifi.annotation.behavior.WritesAttribute; +import org.apache.nifi.annotation.behavior.WritesAttributes; +import org.apache.nifi.annotation.documentation.CapabilityDescription; +import org.apache.nifi.annotation.documentation.SeeAlso; +import org.apache.nifi.annotation.documentation.Tags; +import org.apache.nifi.annotation.lifecycle.OnScheduled; +import org.apache.nifi.components.PropertyDescriptor; +import org.apache.nifi.components.PropertyDescriptor.Builder; +import org.apache.nifi.components.ValidationContext; +import org.apache.nifi.components.ValidationResult; +import org.apache.nifi.flowfile.FlowFile; +import org.apache.nifi.processor.AbstractProcessor; +import org.apache.nifi.processor.ProcessContext; +import org.apache.nifi.processor.ProcessSession; +import org.apache.nifi.processor.Relationship; +import org.apache.nifi.processor.exception.ProcessException; +import org.apache.nifi.schema.access.SchemaNotFoundException; +import org.apache.nifi.script.ScriptingComponentHelper; +import org.apache.nifi.script.ScriptingComponentUtils; +import org.apache.nifi.search.SearchContext; +import org.apache.nifi.search.SearchResult; +import org.apache.nifi.search.Searchable; +import org.apache.nifi.serialization.MalformedRecordException; +import org.apache.nifi.serialization.RecordReader; +import org.apache.nifi.serialization.RecordReaderFactory; +import org.apache.nifi.serialization.RecordSetWriter; +import org.apache.nifi.serialization.RecordSetWriterFactory; +import org.apache.nifi.serialization.WriteResult; +import org.apache.nifi.serialization.record.Record; +import org.python.jsr223.PyScriptEngine; + +import javax.script.Bindings; +import javax.script.CompiledScript; +import javax.script.ScriptContext; +import javax.script.ScriptEngine; +import javax.script.ScriptException; +import javax.script.SimpleBindings; +import java.io.FileInputStream; +import java.io.IOException; +import java.nio.charset.Charset; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.atomic.AtomicLong; + +@EventDriven +@SupportsBatching +@SideEffectFree +@InputRequirement(InputRequirement.Requirement.INPUT_REQUIRED) +@Tags({"record", "transform", "script", "groovy", "jython", "python", "update", "modify", "filter"}) +@WritesAttributes({ +@WritesAttribute(attribute = "mime.type", description = "Sets the mime.type attribute to the MIME Type specified by the Record Writer"), +@WritesAttribute(attribute = "record.count", description = "The number of records in the FlowFile"), +@WritesAttribute(attribute = "record.error.message", description = "This attribute provides on failure the error message encountered by the Reader or Writer.") +}) +@CapabilityDescription("Provides the ability to evaluate a simple script against each record in an incoming FlowFile. The script may transform the record in some way, filter the record, or fork " + +"additional records. See Processor's Additional Details for more information.") +@SeeAlso(classNames = {"org.apache.nifi.processors.script.ExecuteScript", +"org.apache.nifi.processors.standard.UpdateRecord", +"org.apache.nifi.processors.standard.QueryRecord", +"org.apache.nifi.processors.standard.JoltTransformRecord", +"org.apache.nifi.processors.standard.LookupRecord"}) +public class ScriptedTransformRecord extends AbstractPro
[GitHub] [nifi-minifi] alopresto closed pull request #191: MINIFI-532: Support more configurable properties for content and provenance repositories
alopresto closed pull request #191: URL: https://github.com/apache/nifi-minifi/pull/191 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: us...@infra.apache.org
[GitHub] [nifi-minifi] alopresto commented on pull request #191: MINIFI-532: Support more configurable properties for content and provenance repositories
alopresto commented on pull request #191: URL: https://github.com/apache/nifi-minifi/pull/191#issuecomment-652535952 Ran `contrib-check` and all tests pass. +1, merging. 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: us...@infra.apache.org
[GitHub] [nifi-minifi] alopresto commented on pull request #191: MINIFI-532: Support more configurable properties for content and provenance repositories
alopresto commented on pull request #191: URL: https://github.com/apache/nifi-minifi/pull/191#issuecomment-652532672 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: us...@infra.apache.org
[GitHub] [nifi-minifi] mattyb149 opened a new pull request #191: MINIFI-532: Support more configurable properties for content and provenance repositories
mattyb149 opened a new pull request #191: URL: https://github.com/apache/nifi-minifi/pull/191 Thank you for submitting a contribution to Apache NiFi - MiNiFi. 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 MINIFI- where 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 master)? - [x] Is your initial contribution a single, squashed commit? ### 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-minifi folder? - [x] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](https://www.apache.org/legal/resolved.html#category-a)? - [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under minifi-assembly? - [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under minifi-assembly? ### 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 travis-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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Updated] (NIFI-7592) Allow NiFi to be started without a GUI/REST interface
[ https://issues.apache.org/jira/browse/NIFI-7592?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Matt Burgess updated NIFI-7592: --- Status: Patch Available (was: In Progress) > Allow NiFi to be started without a GUI/REST interface > - > > Key: NIFI-7592 > URL: https://issues.apache.org/jira/browse/NIFI-7592 > Project: Apache NiFi > Issue Type: Improvement > Components: Core Framework >Reporter: Matt Burgess >Assignee: Matt Burgess >Priority: Major > Time Spent: 10m > Remaining Estimate: 0h > > In conjunction with MINIFI-422 (bringing MiNiFi into the NiFi codebase), it > would be necessary to allow a NiFi build to run without having the GUI and > REST API components required. For normal NiFi releases, the GUI would be > included in the assembly, but this Jira proposes to reorganize and refactor > the framework code to allow NiFi to run flows and such without requiring the > web bundle. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] tpalfy commented on a change in pull request #4349: NIFI-7549 Adding Hazelcast based DistributedMapCacheClient support
tpalfy commented on a change in pull request #4349: URL: https://github.com/apache/nifi/pull/4349#discussion_r447104375 ## File path: nifi-nar-bundles/nifi-hazelcast-bundle/nifi-hazelcast-services/src/main/java/org/apache/nifi/hazelcast/services/cachemanager/IMapBasedHazelcastCacheManager.java ## @@ -0,0 +1,78 @@ +/* + * 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. + */ +package org.apache.nifi.hazelcast.services.cachemanager; + +import com.hazelcast.core.HazelcastException; +import com.hazelcast.core.HazelcastInstance; +import org.apache.nifi.annotation.lifecycle.OnDisabled; +import org.apache.nifi.annotation.lifecycle.OnEnabled; +import org.apache.nifi.components.PropertyDescriptor; +import org.apache.nifi.components.Validator; +import org.apache.nifi.controller.AbstractControllerService; +import org.apache.nifi.controller.ConfigurationContext; +import org.apache.nifi.expression.ExpressionLanguageScope; +import org.apache.nifi.hazelcast.services.cache.HazelcastCache; +import org.apache.nifi.hazelcast.services.cache.IMapBasedHazelcastCache; +import org.apache.nifi.reporting.InitializationException; + +abstract class IMapBasedHazelcastCacheManager extends AbstractControllerService implements HazelcastCacheManager { +public static final PropertyDescriptor HAZELCAST_INSTANCE_NAME = new PropertyDescriptor.Builder() +.name("hazelcast-instance-name") +.displayName("Hazelcast Instance Name") +.description("Name of the embedded Hazelcast instance") +.required(true) +.addValidator(Validator.VALID) + .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY) +.build(); + +public static final PropertyDescriptor HAZELCAST_CLUSTER_NAME = new PropertyDescriptor.Builder() +.name("hazelcast-cluster-name") +.displayName("Hazelcast Cluster Name") +.description("Name of the embedded Hazelcast instance's cluster") Review comment: `embedded` in this abstract class? ## File path: nifi-nar-bundles/nifi-hazelcast-bundle/nifi-hazelcast-services/src/main/java/org/apache/nifi/hazelcast/services/cachemanager/IMapBasedHazelcastCacheManager.java ## @@ -0,0 +1,78 @@ +/* + * 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. + */ +package org.apache.nifi.hazelcast.services.cachemanager; + +import com.hazelcast.core.HazelcastException; +import com.hazelcast.core.HazelcastInstance; +import org.apache.nifi.annotation.lifecycle.OnDisabled; +import org.apache.nifi.annotation.lifecycle.OnEnabled; +import org.apache.nifi.components.PropertyDescriptor; +import org.apache.nifi.components.Validator; +import org.apache.nifi.controller.AbstractControllerService; +import org.apache.nifi.controller.ConfigurationContext; +import org.apache.nifi.expression.ExpressionLanguageScope; +import org.apache.nifi.hazelcast.services.cache.HazelcastCache; +import org.apache.nifi.hazelcast.services.cache.IMapBasedHazelcastCache; +import org.apache.nifi.reporting.InitializationException; + +abstract class IMapBasedHazelcastCacheManager extends AbstractControllerService implements HazelcastCacheManager { +public static final PropertyDescriptor HAZELCAST_INSTANCE_NAME = new PropertyDescriptor.Builder() +.name("hazelcast-instance-name") +.displayName("Hazelcast Instance Name") +.description("Name of the embedded Hazelcast instance") +.require
[GitHub] [nifi] tpalfy opened a new pull request #4379: NIFI-7594 Clean temporary files in HandleHttpRequest
tpalfy opened a new pull request #4379: URL: https://github.com/apache/nifi/pull/4379 In HandleHttpRequest deleting http request part file resources after flowfile has been transferred. ### 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-** where 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 `master`)? - [ ] 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Created] (NIFI-7594) HadleHttpRequest doesn't clean temporary files
Tamas Palfy created NIFI-7594: - Summary: HadleHttpRequest doesn't clean temporary files Key: NIFI-7594 URL: https://issues.apache.org/jira/browse/NIFI-7594 Project: Apache NiFi Issue Type: Improvement Reporter: Tamas Palfy Assignee: Tamas Palfy The HandleHttpRequest processor stores multipart messages in the temporary directory defined by the "java.io.tmpdir" system property. These files remain there (until the jvm is gracefully shut down). As NiFi is designed to run for long periods of time we should clean up these temporary files after the content has been processed and the flowfile has been transferred. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (NIFI-7593) Azure Blob Storage uses sensitive property for account name
Malthe Borch created NIFI-7593: -- Summary: Azure Blob Storage uses sensitive property for account name Key: NIFI-7593 URL: https://issues.apache.org/jira/browse/NIFI-7593 Project: Apache NiFi Issue Type: Improvement Reporter: Malthe Borch Processors such as [AzureListBlobStorage|https://nifi.apache.org/docs/nifi-docs/components/org.apache.nifi/nifi-azure-nar/1.5.0/org.apache.nifi.processors.azure.storage.ListAzureBlobStorage/] has account name as a sensitive property. This seems wrong and also causes some usability issues with parameter context parameters (which are either sensitive or non-sensitive). Note that while the fix for this is trivial, it could have real implications for existing flows. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #807: MINIFICPP-1228 - Resource ownership refactor + flowFile-owning processors.
adamdebreceni commented on a change in pull request #807: URL: https://github.com/apache/nifi-minifi-cpp/pull/807#discussion_r448289515 ## File path: libminifi/include/core/FlowFile.h ## @@ -30,9 +30,56 @@ namespace minifi { namespace core { class FlowFile : public core::Connectable, public ReferenceContainer { + private: + class FlowFileOwnedResourceClaimPtr{ + public: +FlowFileOwnedResourceClaimPtr() = default; +explicit FlowFileOwnedResourceClaimPtr(const std::shared_ptr& claim) : claim_(claim) { + if (claim_) claim_->increaseFlowFileRecordOwnedCount(); +} +explicit FlowFileOwnedResourceClaimPtr(std::shared_ptr&& claim) : claim_(std::move(claim)) { + if (claim_) claim_->increaseFlowFileRecordOwnedCount(); +} +FlowFileOwnedResourceClaimPtr(const FlowFileOwnedResourceClaimPtr& ref) : claim_(ref.claim_) { + if (claim_) claim_->increaseFlowFileRecordOwnedCount(); +} +FlowFileOwnedResourceClaimPtr(FlowFileOwnedResourceClaimPtr&& ref) : claim_(std::move(ref.claim_)) { + // taking ownership of claim, no need to increment/decrement +} +FlowFileOwnedResourceClaimPtr& operator=(const FlowFileOwnedResourceClaimPtr& ref) = delete; +FlowFileOwnedResourceClaimPtr& operator=(FlowFileOwnedResourceClaimPtr&& ref) = delete; + +FlowFileOwnedResourceClaimPtr& set(FlowFile& owner, const FlowFileOwnedResourceClaimPtr& ref) { + return set(owner, ref.claim_); +} +FlowFileOwnedResourceClaimPtr& set(FlowFile& owner, const std::shared_ptr& newClaim) { + auto oldClaim = claim_; + claim_ = newClaim; + // the order of increase/release is important + if (claim_) claim_->increaseFlowFileRecordOwnedCount(); + if (oldClaim) owner.releaseClaim(oldClaim); + return *this; +} +const std::shared_ptr& get() const { + return claim_; +} +const std::shared_ptr& operator->() const { + return claim_; +} +operator bool() const noexcept { + return static_cast(claim_); +} +~FlowFileOwnedResourceClaimPtr() { + // allow the owner FlowFile to manually release the claim + // while logging stuff and removing it from repositories + assert(!claim_); +} + private: +std::shared_ptr claim_; + }; public: FlowFile(); - ~FlowFile(); + virtual ~FlowFile(); Review comment: done 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: us...@infra.apache.org
[jira] [Commented] (NIFI-7563) Optimize the usage of JMS sessions and message producers
[ https://issues.apache.org/jira/browse/NIFI-7563?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17149325#comment-17149325 ] Gardella Juan Pablo commented on NIFI-7563: --- [~jfrazee] all your suggestions were included at https://github.com/apache/nifi/pull/4378. Sorry about using another PR. I had problems with the rebase. > Optimize the usage of JMS sessions and message producers > > > Key: NIFI-7563 > URL: https://issues.apache.org/jira/browse/NIFI-7563 > Project: Apache NiFi > Issue Type: Improvement > Components: Extensions >Affects Versions: 1.6.0, 1.8.0, 1.7.1, 1.10.0, 1.9.2, 1.11.4 >Reporter: Gardella Juan Pablo >Assignee: Gardella Juan Pablo >Priority: Minor > Time Spent: 27h 10m > Remaining Estimate: 45h > > Below an scenario to reproduce the non optimize usage of JMS resources. > Suppose it is required to publish 1 message to the destination {{D}} using > [PublishJMS|http://nifi.apache.org/docs/nifi-docs/components/org.apache.nifi/nifi-jms-processors-nar/1.11.4/org.apache.nifi.jms.processors.PublishJMS/index.html]. > The message is a flow file in the processor input queue. > It is important to know that internally the processor is using > [CachingConnectionFactory|https://github.com/spring-projects/spring-framework/blob/master/spring-jms/src/main/java/org/springframework/jms/connection/CachingConnectionFactory.java] > to reuse objects and a > [worker|https://github.com/apache/nifi/blob/a1b245e051245bb6c65e7b5ffc6ee982669b7ab7/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/AbstractJMSProcessor.java#L180] > to be able to use in thread safe manner. For JMS publishers, the default > configuration is to cache connections, sessions (only 1) and message > producers. > *Preconditions* > # Flowfile has either {{jms_destination}} or {{jms_replyTo}} attribute > defined. Due to NIFI-7561, it should contain the word {{queue}} or {{topic}}. > Also notice {{jms_destination}} should be ignored, as suggested at NIFI-7564. > That will limit the scenario only when {{jms_replyTo}} attribute is defined. > # For simplicity, the processor is the first time it processes messages. > *Scenario* > # Processor picks the message. The > [worker|https://github.com/apache/nifi/blob/a1b245e051245bb6c65e7b5ffc6ee982669b7ab7/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/AbstractJMSProcessor.java#L180] > is created. > # Connection {{C1}} and session {{S1}} are created. The > [Message|https://docs.oracle.com/javaee/7/api/javax/jms/Message.html] > {{M1_S1}} is created and > [MessageProducer|https://docs.oracle.com/javaee/7/api/javax/jms/MessageProducer.html] > {{MP_S1}} created too. Required to deliver first message at > [JMSPublisher#publish|https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/JMSPublisher.java#L65]. > # S1 and C1 are stored in {{CachingConnectionFactory}}. The caching > connection factory is created at > [AbstractJMSProcessor.java#L208|https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/AbstractJMSProcessor.java#L208]. > # An attempt to create a new connection and a new session are requested to > the connection factory to create destination defined in the header > {{jms_destination}} at > [JMSPublisher.java#L131|https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/JMSPublisher.java#L131]. > Notice the connection {{C1}} is reused although *{{S1}} is not reused* (it > is required to check internal logic in CachingConnectionFactory to understand > why not). A new session {{S2}} is created and stored in the > {{CachingConnectionFactory}} as the new cached session. > # Message is published and {{S1}} and {{MP_S1}} are closed. As {{S1}} is not > in the cache, it is physically closed and {{MP_S1}}. > # At this point of time, the cached objects are {{C1}}, {{S2}}. *Ideally*, > all resources should be reused. > The scenario if it is applied to N consecutive messages create a lot of > sessions and message producers. > We found this issue by adding an > [Interceptor|https://activemq.apache.org/interceptors] to an [Apache ActiveMQ > v5.x|http://activemq.apache.org/components/classic/] broker to detect the > optimal usage of resources. For example, only one message producer per > connection. In below scenario we will be created N producers for the same > connection. Also in a Nifi flow that connects a > [ConsumeJMS|https://nifi.apache.org/docs/nifi-docs/component
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #807: MINIFICPP-1228 - Resource ownership refactor + flowFile-owning processors.
szaszm commented on a change in pull request #807: URL: https://github.com/apache/nifi-minifi-cpp/pull/807#discussion_r448277514 ## File path: libminifi/include/core/FlowFile.h ## @@ -30,9 +30,56 @@ namespace minifi { namespace core { class FlowFile : public core::Connectable, public ReferenceContainer { + private: + class FlowFileOwnedResourceClaimPtr{ + public: +FlowFileOwnedResourceClaimPtr() = default; +explicit FlowFileOwnedResourceClaimPtr(const std::shared_ptr& claim) : claim_(claim) { + if (claim_) claim_->increaseFlowFileRecordOwnedCount(); +} +explicit FlowFileOwnedResourceClaimPtr(std::shared_ptr&& claim) : claim_(std::move(claim)) { + if (claim_) claim_->increaseFlowFileRecordOwnedCount(); +} +FlowFileOwnedResourceClaimPtr(const FlowFileOwnedResourceClaimPtr& ref) : claim_(ref.claim_) { + if (claim_) claim_->increaseFlowFileRecordOwnedCount(); +} +FlowFileOwnedResourceClaimPtr(FlowFileOwnedResourceClaimPtr&& ref) : claim_(std::move(ref.claim_)) { + // taking ownership of claim, no need to increment/decrement +} +FlowFileOwnedResourceClaimPtr& operator=(const FlowFileOwnedResourceClaimPtr& ref) = delete; +FlowFileOwnedResourceClaimPtr& operator=(FlowFileOwnedResourceClaimPtr&& ref) = delete; + +FlowFileOwnedResourceClaimPtr& set(FlowFile& owner, const FlowFileOwnedResourceClaimPtr& ref) { + return set(owner, ref.claim_); +} +FlowFileOwnedResourceClaimPtr& set(FlowFile& owner, const std::shared_ptr& newClaim) { + auto oldClaim = claim_; + claim_ = newClaim; + // the order of increase/release is important + if (claim_) claim_->increaseFlowFileRecordOwnedCount(); + if (oldClaim) owner.releaseClaim(oldClaim); + return *this; +} +const std::shared_ptr& get() const { + return claim_; +} +const std::shared_ptr& operator->() const { + return claim_; +} +operator bool() const noexcept { + return static_cast(claim_); +} +~FlowFileOwnedResourceClaimPtr() { + // allow the owner FlowFile to manually release the claim + // while logging stuff and removing it from repositories + assert(!claim_); +} + private: +std::shared_ptr claim_; + }; public: FlowFile(); - ~FlowFile(); + virtual ~FlowFile(); Review comment: The supporting guideline, only referring to "virtual functions" without mentioning destructors: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-override Another one in which the example suggests that destructor is not excluded from the above rule: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c130-for-making-deep-copies-of-polymorphic-classes-prefer-a-virtual-clone-function-instead-of-copy-constructionassignment Based on these, my standpoint remains. A logical rationale could be that a destructor definition implicitly calls the base class destructors after its execution, so it's actually overriding the base class implementation with a new one that happens to call the base class implementation at the end. 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: us...@infra.apache.org
[jira] [Updated] (NIFI-7563) Optimize the usage of JMS sessions and message producers
[ https://issues.apache.org/jira/browse/NIFI-7563?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Gardella Juan Pablo updated NIFI-7563: -- Latest PR at https://github.com/apache/nifi/pull/4378 > Optimize the usage of JMS sessions and message producers > > > Key: NIFI-7563 > URL: https://issues.apache.org/jira/browse/NIFI-7563 > Project: Apache NiFi > Issue Type: Improvement > Components: Extensions >Affects Versions: 1.6.0, 1.8.0, 1.7.1, 1.10.0, 1.9.2, 1.11.4 >Reporter: Gardella Juan Pablo >Assignee: Gardella Juan Pablo >Priority: Minor > Time Spent: 25h 10m > Remaining Estimate: 47h > > Below an scenario to reproduce the non optimize usage of JMS resources. > Suppose it is required to publish 1 message to the destination {{D}} using > [PublishJMS|http://nifi.apache.org/docs/nifi-docs/components/org.apache.nifi/nifi-jms-processors-nar/1.11.4/org.apache.nifi.jms.processors.PublishJMS/index.html]. > The message is a flow file in the processor input queue. > It is important to know that internally the processor is using > [CachingConnectionFactory|https://github.com/spring-projects/spring-framework/blob/master/spring-jms/src/main/java/org/springframework/jms/connection/CachingConnectionFactory.java] > to reuse objects and a > [worker|https://github.com/apache/nifi/blob/a1b245e051245bb6c65e7b5ffc6ee982669b7ab7/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/AbstractJMSProcessor.java#L180] > to be able to use in thread safe manner. For JMS publishers, the default > configuration is to cache connections, sessions (only 1) and message > producers. > *Preconditions* > # Flowfile has either {{jms_destination}} or {{jms_replyTo}} attribute > defined. Due to NIFI-7561, it should contain the word {{queue}} or {{topic}}. > Also notice {{jms_destination}} should be ignored, as suggested at NIFI-7564. > That will limit the scenario only when {{jms_replyTo}} attribute is defined. > # For simplicity, the processor is the first time it processes messages. > *Scenario* > # Processor picks the message. The > [worker|https://github.com/apache/nifi/blob/a1b245e051245bb6c65e7b5ffc6ee982669b7ab7/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/AbstractJMSProcessor.java#L180] > is created. > # Connection {{C1}} and session {{S1}} are created. The > [Message|https://docs.oracle.com/javaee/7/api/javax/jms/Message.html] > {{M1_S1}} is created and > [MessageProducer|https://docs.oracle.com/javaee/7/api/javax/jms/MessageProducer.html] > {{MP_S1}} created too. Required to deliver first message at > [JMSPublisher#publish|https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/JMSPublisher.java#L65]. > # S1 and C1 are stored in {{CachingConnectionFactory}}. The caching > connection factory is created at > [AbstractJMSProcessor.java#L208|https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/AbstractJMSProcessor.java#L208]. > # An attempt to create a new connection and a new session are requested to > the connection factory to create destination defined in the header > {{jms_destination}} at > [JMSPublisher.java#L131|https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/JMSPublisher.java#L131]. > Notice the connection {{C1}} is reused although *{{S1}} is not reused* (it > is required to check internal logic in CachingConnectionFactory to understand > why not). A new session {{S2}} is created and stored in the > {{CachingConnectionFactory}} as the new cached session. > # Message is published and {{S1}} and {{MP_S1}} are closed. As {{S1}} is not > in the cache, it is physically closed and {{MP_S1}}. > # At this point of time, the cached objects are {{C1}}, {{S2}}. *Ideally*, > all resources should be reused. > The scenario if it is applied to N consecutive messages create a lot of > sessions and message producers. > We found this issue by adding an > [Interceptor|https://activemq.apache.org/interceptors] to an [Apache ActiveMQ > v5.x|http://activemq.apache.org/components/classic/] broker to detect the > optimal usage of resources. For example, only one message producer per > connection. In below scenario we will be created N producers for the same > connection. Also in a Nifi flow that connects a > [ConsumeJMS|https://nifi.apache.org/docs/nifi-docs/components/org.apache.nifi/nifi-jms-processors-nar/1.11.4/org.apache.nifi.jms.processors.ConsumeJMS/] > with a PublishJMS. Notice {{ConsumeJMS}} populates by default
[GitHub] [nifi] gardellajuanpablo closed pull request #4352: NIFI-7563 Optimize the usage of JMS sessions and message producers
gardellajuanpablo closed pull request #4352: URL: https://github.com/apache/nifi/pull/4352 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: us...@infra.apache.org
[GitHub] [nifi] gardellajuanpablo commented on pull request #4352: NIFI-7563 Optimize the usage of JMS sessions and message producers
gardellajuanpablo commented on pull request #4352: URL: https://github.com/apache/nifi/pull/4352#issuecomment-652323183 Please cancel this PR 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: us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #807: MINIFICPP-1228 - Resource ownership refactor + flowFile-owning processors.
adamdebreceni commented on a change in pull request #807: URL: https://github.com/apache/nifi-minifi-cpp/pull/807#discussion_r448255811 ## File path: libminifi/test/BufferReader.h ## @@ -0,0 +1,51 @@ +/** + * + * 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. + */ + +#ifndef NIFI_MINIFI_CPP_BUFFERREADER_H +#define NIFI_MINIFI_CPP_BUFFERREADER_H + +#include "FlowFileRecord.h" + +class BufferReader : public org::apache::nifi::minifi::InputStreamCallback { + public: + explicit BufferReader(std::vector& buffer) : buffer_(buffer){} + template + int write(Input input, std::size_t len) { Review comment: done (I don't remember what I was planning with the template) 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: us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #807: MINIFICPP-1228 - Resource ownership refactor + flowFile-owning processors.
adamdebreceni commented on a change in pull request #807: URL: https://github.com/apache/nifi-minifi-cpp/pull/807#discussion_r448255811 ## File path: libminifi/test/BufferReader.h ## @@ -0,0 +1,51 @@ +/** + * + * 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. + */ + +#ifndef NIFI_MINIFI_CPP_BUFFERREADER_H +#define NIFI_MINIFI_CPP_BUFFERREADER_H + +#include "FlowFileRecord.h" + +class BufferReader : public org::apache::nifi::minifi::InputStreamCallback { + public: + explicit BufferReader(std::vector& buffer) : buffer_(buffer){} + template + int write(Input input, std::size_t len) { Review comment: done (I don't remember what was I planning with the template) 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: us...@infra.apache.org
[GitHub] [nifi] gardellajuanpablo commented on a change in pull request #4352: NIFI-7563 Optimize the usage of JMS sessions and message producers
gardellajuanpablo commented on a change in pull request #4352: URL: https://github.com/apache/nifi/pull/4352#discussion_r448255708 ## File path: nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/test/java/org/apache/nifi/jms/processors/PublishJMSIT.java ## @@ -385,6 +390,66 @@ protected TcpTransport createTcpTransport(WireFormat wf, SocketFactory socketFac } } +/** + * + * This test validates the optimal resources usage. To process one message is expected to create only one connection, one session and one message producer. + * + * + * See https://issues.apache.org/jira/browse/NIFI-7563 for details. + * + * @throws Exception any error related to the broker. + */ +@Test(timeout = 1) +public void validateNIFI7563() throws Exception { +BrokerService broker = new BrokerService(); +try { +broker.setPersistent(false); +TransportConnector connector = broker.addConnector("tcp://127.0.0.1:0"); +int port = connector.getServer().getSocketAddress().getPort(); +broker.start(); + +final ActiveMQConnectionFactory innerCf = new ActiveMQConnectionFactory("tcp://127.0.0.1:" + port); +ConnectionFactoryInvocationHandler connectionFactoryProxy = new ConnectionFactoryInvocationHandler(innerCf); + +// Create a connection Factory proxy to catch metrics and usage. +ConnectionFactory cf = (ConnectionFactory) Proxy.newProxyInstance(ConnectionFactory.class.getClassLoader(), new Class[] { ConnectionFactory.class }, connectionFactoryProxy); + +TestRunner runner = TestRunners.newTestRunner(new PublishJMS()); +JMSConnectionFactoryProviderDefinition cs = mock(JMSConnectionFactoryProviderDefinition.class); +when(cs.getIdentifier()).thenReturn("cfProvider"); +when(cs.getConnectionFactory()).thenReturn(cf); +runner.addControllerService("cfProvider", cs); +runner.enableControllerService(cs); + +runner.setProperty(PublishJMS.CF_SERVICE, "cfProvider"); + +String destinationName = "myDestinationName"; +// The destination option according current implementation should contain topic or queue to infer the destination type +// from the name. Check https://issues.apache.org/jira/browse/NIFI-7561. Once that is fixed, the name can be +// randomly created. +String topicNameInHeader = "topic-foo"; +runner.setProperty(PublishJMS.DESTINATION, destinationName); +runner.setProperty(PublishJMS.DESTINATION_TYPE, PublishJMS.QUEUE); + +Map flowFileAttributes = new HashMap<>(); +// This method will be removed once https://issues.apache.org/jira/browse/NIFI-7564 is fixed. +flowFileAttributes.put(JmsHeaders.DESTINATION, topicNameInHeader); +flowFileAttributes.put(JmsHeaders.REPLY_TO, topicNameInHeader); +runner.enqueue("hi".getBytes(), flowFileAttributes); +runner.enqueue("h2".getBytes(), flowFileAttributes); +runner.setThreadCount(1); Review comment: Done, but I messed up during rebase. I have to create another branch from latest changes in master. The changes are now at https://github.com/apache/nifi/pull/4378. Sorry about that, could you please review it soon? That will prevent to me rebase it again :). 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: us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #807: MINIFICPP-1228 - Resource ownership refactor + flowFile-owning processors.
adamdebreceni commented on a change in pull request #807: URL: https://github.com/apache/nifi-minifi-cpp/pull/807#discussion_r448254975 ## File path: libminifi/test/BufferReader.h ## @@ -0,0 +1,51 @@ +/** + * + * 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. + */ + +#ifndef NIFI_MINIFI_CPP_BUFFERREADER_H +#define NIFI_MINIFI_CPP_BUFFERREADER_H + +#include "FlowFileRecord.h" + +class BufferReader : public org::apache::nifi::minifi::InputStreamCallback { + public: + explicit BufferReader(std::vector& buffer) : buffer_(buffer){} + template + int write(Input input, std::size_t len) { +uint8_t tmpBuffer[4096]{}; +int total_read = 0; +do { + auto ret = input.read(tmpBuffer, std::min(len, sizeof(tmpBuffer))); + if (ret == 0) break; + if (ret < 0) return ret; + len -= ret; Review comment: done 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: us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #807: MINIFICPP-1228 - Resource ownership refactor + flowFile-owning processors.
adamdebreceni commented on a change in pull request #807: URL: https://github.com/apache/nifi-minifi-cpp/pull/807#discussion_r448255306 ## File path: libminifi/test/BufferReader.h ## @@ -0,0 +1,51 @@ +/** + * + * 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. + */ + +#ifndef NIFI_MINIFI_CPP_BUFFERREADER_H +#define NIFI_MINIFI_CPP_BUFFERREADER_H + +#include "FlowFileRecord.h" + +class BufferReader : public org::apache::nifi::minifi::InputStreamCallback { + public: + explicit BufferReader(std::vector& buffer) : buffer_(buffer){} + template + int write(Input input, std::size_t len) { +uint8_t tmpBuffer[4096]{}; +int total_read = 0; +do { + auto ret = input.read(tmpBuffer, std::min(len, sizeof(tmpBuffer))); + if (ret == 0) break; + if (ret < 0) return ret; + len -= ret; + total_read += ret; + auto prevSize = buffer_.size(); + buffer_.resize(prevSize + ret); + std::move(tmpBuffer, tmpBuffer + ret, buffer_.data() + prevSize); +} while (len); Review comment: done 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: us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #807: MINIFICPP-1228 - Resource ownership refactor + flowFile-owning processors.
adamdebreceni commented on a change in pull request #807: URL: https://github.com/apache/nifi-minifi-cpp/pull/807#discussion_r448254564 ## File path: libminifi/include/core/ContentRepository.h ## @@ -104,7 +104,7 @@ class ContentRepository : public StreamManager { if (count != count_map_.end() && count->second > 0) { count_map_[str] = count->second - 1; } else { - count_map_.erase(str); + count_map_.erase(str); Review comment: done ## File path: extensions/libarchive/BinFiles.cpp ## @@ -70,6 +71,8 @@ void BinFiles::initialize() { relationships.insert(Original); relationships.insert(Failure); setSupportedRelationships(relationships); + + out_going_connections_[Self.getName()].insert(shared_from_this()); Review comment: done 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: us...@infra.apache.org
[GitHub] [nifi] gardellajuanpablo opened a new pull request #4378: Nifi7563
gardellajuanpablo opened a new pull request #4378: URL: https://github.com/apache/nifi/pull/4378 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-._ 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-** where 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 `master`)? - [ ] 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #807: MINIFICPP-1228 - Resource ownership refactor + flowFile-owning processors.
adamdebreceni commented on a change in pull request #807: URL: https://github.com/apache/nifi-minifi-cpp/pull/807#discussion_r448208397 ## File path: libminifi/include/core/FlowFile.h ## @@ -30,9 +30,56 @@ namespace minifi { namespace core { class FlowFile : public core::Connectable, public ReferenceContainer { + private: + class FlowFileOwnedResourceClaimPtr{ + public: +FlowFileOwnedResourceClaimPtr() = default; +explicit FlowFileOwnedResourceClaimPtr(const std::shared_ptr& claim) : claim_(claim) { + if (claim_) claim_->increaseFlowFileRecordOwnedCount(); +} +explicit FlowFileOwnedResourceClaimPtr(std::shared_ptr&& claim) : claim_(std::move(claim)) { + if (claim_) claim_->increaseFlowFileRecordOwnedCount(); +} +FlowFileOwnedResourceClaimPtr(const FlowFileOwnedResourceClaimPtr& ref) : claim_(ref.claim_) { + if (claim_) claim_->increaseFlowFileRecordOwnedCount(); +} +FlowFileOwnedResourceClaimPtr(FlowFileOwnedResourceClaimPtr&& ref) : claim_(std::move(ref.claim_)) { + // taking ownership of claim, no need to increment/decrement +} +FlowFileOwnedResourceClaimPtr& operator=(const FlowFileOwnedResourceClaimPtr& ref) = delete; +FlowFileOwnedResourceClaimPtr& operator=(FlowFileOwnedResourceClaimPtr&& ref) = delete; + +FlowFileOwnedResourceClaimPtr& set(FlowFile& owner, const FlowFileOwnedResourceClaimPtr& ref) { + return set(owner, ref.claim_); +} +FlowFileOwnedResourceClaimPtr& set(FlowFile& owner, const std::shared_ptr& newClaim) { + auto oldClaim = claim_; + claim_ = newClaim; + // the order of increase/release is important + if (claim_) claim_->increaseFlowFileRecordOwnedCount(); + if (oldClaim) owner.releaseClaim(oldClaim); + return *this; +} +const std::shared_ptr& get() const { + return claim_; +} +const std::shared_ptr& operator->() const { + return claim_; +} +operator bool() const noexcept { + return static_cast(claim_); +} +~FlowFileOwnedResourceClaimPtr() { + // allow the owner FlowFile to manually release the claim + // while logging stuff and removing it from repositories + assert(!claim_); +} + private: +std::shared_ptr claim_; + }; public: FlowFile(); - ~FlowFile(); + virtual ~FlowFile(); Review comment: since virtual destructors have different semantics from other virtual functions (they never actually override the base destructor but they are chained) they shouldn't be marked `override` imo 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: us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #807: MINIFICPP-1228 - Resource ownership refactor + flowFile-owning processors.
szaszm commented on a change in pull request #807: URL: https://github.com/apache/nifi-minifi-cpp/pull/807#discussion_r445652913 ## File path: extensions/libarchive/BinFiles.cpp ## @@ -153,7 +156,7 @@ void BinManager::gatherReadyBins() { void BinManager::removeOldestBin() { std::lock_guard < std::mutex > lock(mutex_); uint64_t olddate = ULLONG_MAX; - std::unique_ptr < std::deque>>*oldqueue; + std::unique_ptr < std::deque>>* oldqueue; Review comment: not your issue, but wtf: pointer to a smart ptr to a collection of smart ptrs to Bin. ## File path: libminifi/include/core/ContentRepository.h ## @@ -104,7 +104,7 @@ class ContentRepository : public StreamManager { if (count != count_map_.end() && count->second > 0) { count_map_[str] = count->second - 1; } else { - count_map_.erase(str); + count_map_.erase(str); Review comment: wrong indentation ## File path: libminifi/include/core/FlowFile.h ## @@ -30,9 +30,56 @@ namespace minifi { namespace core { class FlowFile : public core::Connectable, public ReferenceContainer { + private: + class FlowFileOwnedResourceClaimPtr{ + public: +FlowFileOwnedResourceClaimPtr() = default; +explicit FlowFileOwnedResourceClaimPtr(const std::shared_ptr& claim) : claim_(claim) { + if (claim_) claim_->increaseFlowFileRecordOwnedCount(); +} +explicit FlowFileOwnedResourceClaimPtr(std::shared_ptr&& claim) : claim_(std::move(claim)) { + if (claim_) claim_->increaseFlowFileRecordOwnedCount(); +} +FlowFileOwnedResourceClaimPtr(const FlowFileOwnedResourceClaimPtr& ref) : claim_(ref.claim_) { + if (claim_) claim_->increaseFlowFileRecordOwnedCount(); +} +FlowFileOwnedResourceClaimPtr(FlowFileOwnedResourceClaimPtr&& ref) : claim_(std::move(ref.claim_)) { + // taking ownership of claim, no need to increment/decrement +} +FlowFileOwnedResourceClaimPtr& operator=(const FlowFileOwnedResourceClaimPtr& ref) = delete; +FlowFileOwnedResourceClaimPtr& operator=(FlowFileOwnedResourceClaimPtr&& ref) = delete; + +FlowFileOwnedResourceClaimPtr& set(FlowFile& owner, const FlowFileOwnedResourceClaimPtr& ref) { + return set(owner, ref.claim_); +} +FlowFileOwnedResourceClaimPtr& set(FlowFile& owner, const std::shared_ptr& newClaim) { + auto oldClaim = claim_; + claim_ = newClaim; + // the order of increase/release is important + if (claim_) claim_->increaseFlowFileRecordOwnedCount(); + if (oldClaim) owner.releaseClaim(oldClaim); + return *this; +} +const std::shared_ptr& get() const { + return claim_; +} +const std::shared_ptr& operator->() const { + return claim_; +} +operator bool() const noexcept { + return static_cast(claim_); +} +~FlowFileOwnedResourceClaimPtr() { + // allow the owner FlowFile to manually release the claim + // while logging stuff and removing it from repositories + assert(!claim_); +} + private: +std::shared_ptr claim_; + }; public: FlowFile(); - ~FlowFile(); + virtual ~FlowFile(); Review comment: Add `override` instead for clarification. ## File path: libminifi/test/BufferReader.h ## @@ -0,0 +1,51 @@ +/** + * + * 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. + */ + +#ifndef NIFI_MINIFI_CPP_BUFFERREADER_H +#define NIFI_MINIFI_CPP_BUFFERREADER_H + +#include "FlowFileRecord.h" + +class BufferReader : public org::apache::nifi::minifi::InputStreamCallback { + public: + explicit BufferReader(std::vector& buffer) : buffer_(buffer){} + template + int write(Input input, std::size_t len) { +uint8_t tmpBuffer[4096]{}; +int total_read = 0; +do { + auto ret = input.read(tmpBuffer, std::min(len, sizeof(tmpBuffer))); + if (ret == 0) break; + if (ret < 0) return ret; + len -= ret; Review comment: I think the code would be easier to understand if we didn't change the argument, but used a separate `read` or `remaining` variable. The less moving parts, the better. The generated code would most likely be the same. ## File path: libminifi/test/BufferReader.h ##
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #827: MINIFICPP-1273 - Drain connections on flow shutdown
adamdebreceni commented on a change in pull request #827: URL: https://github.com/apache/nifi-minifi-cpp/pull/827#discussion_r448156824 ## File path: libminifi/test/flow-tests/FlowControllerTests.cpp ## @@ -0,0 +1,125 @@ +/** + * + * 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. + */ + +#undef NDEBUG +#include +#include +#include +#include +#include + +#include "core/Core.h" +#include "core/repository/AtomicRepoEntries.h" +#include "core/RepositoryFactory.h" +#include "FlowFileRecord.h" +#include "provenance/Provenance.h" +#include "properties/Configure.h" +#include "../unit/ProvenanceTestHelper.h" +#include "../TestBase.h" +#include "YamlConfiguration.h" + +const char* yamlConfig = Review comment: I saw that a number of tests store their config files there, but the fact that I have to search for the test by name then check the `CMakeLists.txt` to find out what the command line arguments are going to be, and then manually navigate to the file, makes me think that it is not the best approach 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: us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #827: MINIFICPP-1273 - Drain connections on flow shutdown
adamdebreceni commented on a change in pull request #827: URL: https://github.com/apache/nifi-minifi-cpp/pull/827#discussion_r448156824 ## File path: libminifi/test/flow-tests/FlowControllerTests.cpp ## @@ -0,0 +1,125 @@ +/** + * + * 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. + */ + +#undef NDEBUG +#include +#include +#include +#include +#include + +#include "core/Core.h" +#include "core/repository/AtomicRepoEntries.h" +#include "core/RepositoryFactory.h" +#include "FlowFileRecord.h" +#include "provenance/Provenance.h" +#include "properties/Configure.h" +#include "../unit/ProvenanceTestHelper.h" +#include "../TestBase.h" +#include "YamlConfiguration.h" + +const char* yamlConfig = Review comment: I saw that a number of tests store their config files there, but the fact that I have to search for the test by name then check the CMakeLists.txt what the command line arguments are going to be, and then manually navigate to the file, makes me think that it is not the best approach 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: us...@infra.apache.org