[jira] [Created] (NIFI-7138) Consider Github Packages to publish NiFi Docker images
Pierre Villard created NIFI-7138: Summary: Consider Github Packages to publish NiFi Docker images Key: NIFI-7138 URL: https://issues.apache.org/jira/browse/NIFI-7138 Project: Apache NiFi Issue Type: New Feature Components: Docker, Tools and Build Reporter: Pierre Villard Using a combination of Github Actions and Github Packages, it would be possible to build NiFi Docker images and make the images available on the repository. It would also be possible to provide a new image each time a commit is merged into master using [Github CI|[https://help.github.com/en/packages/using-github-packages-with-your-projects-ecosystem/using-github-packages-with-github-actions#publishing-a-package-using-an-action]]. This could also help when reviewing PRs and/or testing release candidates. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (NIFI-7137) Consider Github Actions as a replacement of Travis CI
Pierre Villard created NIFI-7137: Summary: Consider Github Actions as a replacement of Travis CI Key: NIFI-7137 URL: https://issues.apache.org/jira/browse/NIFI-7137 Project: Apache NiFi Issue Type: New Feature Components: Tools and Build Reporter: Pierre Villard It could be interesting to consider using [Github Actions|[https://github.com/features/actions]] instead of Travis CI. This could provide more reliability as well as a better solution to test Windows builds. This has already been adopted by some Apache projects. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] pvillard31 commented on issue #4050: NIFI-7135 - Fix Java 11 build with com.puppycrawl.tools:checkstyle:jar:8.29 dependency
pvillard31 commented on issue #4050: NIFI-7135 - Fix Java 11 build with com.puppycrawl.tools:checkstyle:jar:8.29 dependency URL: https://github.com/apache/nifi/pull/4050#issuecomment-585045353 This does solve the dependency issue but the Java 11 build fails on some tests. Using the Travis OpenJDK 11.0.2_9 instead of AdoptOpenJDK 11.0.4_11 could be the reason... @jtstorck - does it sound familiar? 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 With regards, Apache Git Services
[jira] [Updated] (NIFI-7046) Change travis.yml configuration for sourcing maven download
[ https://issues.apache.org/jira/browse/NIFI-7046?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Pierre Villard updated NIFI-7046: - Assignee: Pierre Villard (was: Joe Witt) Status: Patch Available (was: Open) > Change travis.yml configuration for sourcing maven download > --- > > Key: NIFI-7046 > URL: https://issues.apache.org/jira/browse/NIFI-7046 > Project: Apache NiFi > Issue Type: Task >Reporter: Joe Witt >Assignee: Pierre Villard >Priority: Critical > Time Spent: 10m > Remaining Estimate: 0h > > We should not be getting the maven download from the apache archives. We > should get them from a mirror. > And we should update to the latest maven release which is 3.6.3 as of now > https://github.com/apache/nifi/blob/master/.travis.yml > Change the part at the end to > # Install Maven 3.6.3 > - wget -O /tmp/apache-maven-3.6.3-bin.tar.gz > http://apache.spinellicreations.com/maven/maven-3/3.6.3/binaries/apache-maven-3.6.3-bin.tar.gz > - tar xzf /tmp/apache-maven-3.6.3-bin.tar.gz -C /tmp > - export M2_HOME=/tmp/apache-maven-3.6.3 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] pvillard31 opened a new pull request #4051: NIFI-7046 - upgrade maven in Travis build and download from a mirror
pvillard31 opened a new pull request #4051: NIFI-7046 - upgrade maven in Travis
build and download from a mirror
URL: https://github.com/apache/nifi/pull/4051
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 both JDK 8 and
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 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
With regards,
Apache Git Services
[jira] [Commented] (NIFI-7046) Change travis.yml configuration for sourcing maven download
[ https://issues.apache.org/jira/browse/NIFI-7046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17035058#comment-17035058 ] Pierre Villard commented on NIFI-7046: -- I looked at the mirror supporting https. I'm going to submit a PR using: https://apache.brunneis.com/maven/maven-3/3.6.3/binaries/apache-maven-3.6.3-bin.tar.gz > Change travis.yml configuration for sourcing maven download > --- > > Key: NIFI-7046 > URL: https://issues.apache.org/jira/browse/NIFI-7046 > Project: Apache NiFi > Issue Type: Task >Reporter: Joe Witt >Assignee: Joe Witt >Priority: Critical > > We should not be getting the maven download from the apache archives. We > should get them from a mirror. > And we should update to the latest maven release which is 3.6.3 as of now > https://github.com/apache/nifi/blob/master/.travis.yml > Change the part at the end to > # Install Maven 3.6.3 > - wget -O /tmp/apache-maven-3.6.3-bin.tar.gz > http://apache.spinellicreations.com/maven/maven-3/3.6.3/binaries/apache-maven-3.6.3-bin.tar.gz > - tar xzf /tmp/apache-maven-3.6.3-bin.tar.gz -C /tmp > - export M2_HOME=/tmp/apache-maven-3.6.3 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (NIFI-7136) Set the autocomplete HTML5 tag to false for username/password login fields
M Tien created NIFI-7136: Summary: Set the autocomplete HTML5 tag to false for username/password login fields Key: NIFI-7136 URL: https://issues.apache.org/jira/browse/NIFI-7136 Project: Apache NiFi Issue Type: Bug Affects Versions: 1.11.1 Reporter: M Tien The autocomplete field may need to be set to false for security purposes. Investigate the benefits of setting this to false, and if valid, ensure it's set to false for NiFi. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NIFI-7114) NiFi not closing file handles
[ https://issues.apache.org/jira/browse/NIFI-7114?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17035017#comment-17035017 ] Joe Witt commented on NIFI-7114: ExecuteScript:285 suspicious line. doesnt seem like stream would be closed. though this is a search method so not the likely issue being observed for this case. > NiFi not closing file handles > - > > Key: NIFI-7114 > URL: https://issues.apache.org/jira/browse/NIFI-7114 > Project: Apache NiFi > Issue Type: Bug > Components: Configuration >Affects Versions: 1.10.0, 1.11.0 > Environment: Amazon EC2 running either Amazon Linux 2 or Ubuntu 18.04. > NiFi has been installed with no change to any configuration file. >Reporter: Vinicius Zolin >Priority: Major > Fix For: 1.11.2 > > Attachments: destination.xml, lsof.log, lsof.zip, lsofAfter.log, > lsofBefore.log, openFiles.xlsx, source.xml > > > Since at least version 1.10 NiFi stopped closing file handles. It opens circa > 500 files per hour (measured using lsof) without any apparent limit until it > crashes due to too many open files. > > Increasing the computer open file limit is not a solution since NiFi will > still crash, it'll only take longer to do so. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] pvillard31 commented on issue #4050: NIFI-7135 - Fix Java 11 build with com.puppycrawl.tools:checkstyle:jar:8.29 dependency
pvillard31 commented on issue #4050: NIFI-7135 - Fix Java 11 build with com.puppycrawl.tools:checkstyle:jar:8.29 dependency URL: https://github.com/apache/nifi/pull/4050#issuecomment-585000696 Let's see if Travis is happier with this... 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 With regards, Apache Git Services
[jira] [Updated] (NIFI-7135) Fix Java 11 build with com.puppycrawl.tools:checkstyle:jar:8.29 dependency
[
https://issues.apache.org/jira/browse/NIFI-7135?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Pierre Villard updated NIFI-7135:
-
Status: Patch Available (was: Open)
> Fix Java 11 build with com.puppycrawl.tools:checkstyle:jar:8.29 dependency
> --
>
> Key: NIFI-7135
> URL: https://issues.apache.org/jira/browse/NIFI-7135
> Project: Apache NiFi
> Issue Type: Bug
> Components: Tools and Build
>Reporter: Pierre Villard
>Assignee: Pierre Villard
>Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Since the upgrade of com.puppycrawl.tools:checkstyle to 8.29 (NIFI-7108), the
> Java 11 build on Travis is failing with:
> {code:java}
> [ERROR] Failed to execute goal
> org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0:check (check-style) on
> project nifi: Execution check-style of goal
> org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0:check failed: Plugin
> org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0 or one of its
> dependencies could not be resolved: Could not find artifact
> com.puppycrawl.tools:checkstyle:jar:8.29 in bintray
> (https://dl.bintray.com/groovy/maven) -> [Help 1]{code}
> It looks like plugin dependencies are only resolved against:
> {code:java}
>
>
> bintray
> Groovy Bintray
> https://dl.bintray.com/groovy/maven
>
> never
>
>
> false
>
>
>
> {code}
> I suggest to add Maven repository as an additional plugin repository.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [nifi] pvillard31 opened a new pull request #4050: NIFI-7135 - Fix Java 11 build with com.puppycrawl.tools:checkstyle:jar:8.29 dependency
pvillard31 opened a new pull request #4050: NIFI-7135 - Fix Java 11 build with
com.puppycrawl.tools:checkstyle:jar:8.29 dependency
URL: https://github.com/apache/nifi/pull/4050
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 both JDK 8 and
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 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
With regards,
Apache Git Services
[jira] [Created] (NIFI-7135) Fix Java 11 build with com.puppycrawl.tools:checkstyle:jar:8.29 dependency
Pierre Villard created NIFI-7135:
Summary: Fix Java 11 build with
com.puppycrawl.tools:checkstyle:jar:8.29 dependency
Key: NIFI-7135
URL: https://issues.apache.org/jira/browse/NIFI-7135
Project: Apache NiFi
Issue Type: Bug
Components: Tools and Build
Reporter: Pierre Villard
Assignee: Pierre Villard
Since the upgrade of com.puppycrawl.tools:checkstyle to 8.29 (NIFI-7108), the
Java 11 build on Travis is failing with:
{code:java}
[ERROR] Failed to execute goal
org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0:check (check-style) on
project nifi: Execution check-style of goal
org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0:check failed: Plugin
org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0 or one of its
dependencies could not be resolved: Could not find artifact
com.puppycrawl.tools:checkstyle:jar:8.29 in bintray
(https://dl.bintray.com/groovy/maven) -> [Help 1]{code}
It looks like plugin dependencies are only resolved against:
{code:java}
bintray
Groovy Bintray
https://dl.bintray.com/groovy/maven
never
false
{code}
I suggest to add Maven repository as an additional plugin repository.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NIFI-7114) NiFi not closing file handles
[ https://issues.apache.org/jira/browse/NIFI-7114?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034996#comment-17034996 ] Joe Witt commented on NIFI-7114: The most obvious thing growing over time is entries for 'pipe'. Stack overflow gives us https://stackoverflow.com/questions/15956452/troubleshooting-too-many-files-open-with-lsof I then looked at your destination flow and indeed there are many uses of scripted processors so this is where I'm focusing my review now. Is it that your scripts are opening handles and not closing them or is that we're not closing something on their behalf. That is the current investigation. > NiFi not closing file handles > - > > Key: NIFI-7114 > URL: https://issues.apache.org/jira/browse/NIFI-7114 > Project: Apache NiFi > Issue Type: Bug > Components: Configuration >Affects Versions: 1.10.0, 1.11.0 > Environment: Amazon EC2 running either Amazon Linux 2 or Ubuntu 18.04. > NiFi has been installed with no change to any configuration file. >Reporter: Vinicius Zolin >Priority: Major > Fix For: 1.11.2 > > Attachments: destination.xml, lsof.log, lsof.zip, lsofAfter.log, > lsofBefore.log, openFiles.xlsx, source.xml > > > Since at least version 1.10 NiFi stopped closing file handles. It opens circa > 500 files per hour (measured using lsof) without any apparent limit until it > crashes due to too many open files. > > Increasing the computer open file limit is not a solution since NiFi will > still crash, it'll only take longer to do so. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NIFI-7114) NiFi not closing file handles
[ https://issues.apache.org/jira/browse/NIFI-7114?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034984#comment-17034984 ] Joe Witt commented on NIFI-7114: wonderful. reviewing [~vzolin]. Thanks for your efforts here to help with this > NiFi not closing file handles > - > > Key: NIFI-7114 > URL: https://issues.apache.org/jira/browse/NIFI-7114 > Project: Apache NiFi > Issue Type: Bug > Components: Configuration >Affects Versions: 1.10.0, 1.11.0 > Environment: Amazon EC2 running either Amazon Linux 2 or Ubuntu 18.04. > NiFi has been installed with no change to any configuration file. >Reporter: Vinicius Zolin >Priority: Major > Fix For: 1.11.2 > > Attachments: destination.xml, lsof.log, lsof.zip, lsofAfter.log, > lsofBefore.log, openFiles.xlsx, source.xml > > > Since at least version 1.10 NiFi stopped closing file handles. It opens circa > 500 files per hour (measured using lsof) without any apparent limit until it > crashes due to too many open files. > > Increasing the computer open file limit is not a solution since NiFi will > still crash, it'll only take longer to do so. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NIFI-7064) Support 2-way SSL by InvokeHTTP processor
[
https://issues.apache.org/jira/browse/NIFI-7064?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034877#comment-17034877
]
Andy LoPresto commented on NIFI-7064:
-
Dmitry, I don't understand your last question. Can you point to a specific line
where you feel the context is being "rejected" in {{InvokeHTTP}}?
> Support 2-way SSL by InvokeHTTP processor
> -
>
> Key: NIFI-7064
> URL: https://issues.apache.org/jira/browse/NIFI-7064
> Project: Apache NiFi
> Issue Type: Improvement
> Components: Core Framework, Security
>Affects Versions: 1.10.0
>Reporter: Dmitry Mashkov
>Priority: Major
> Labels: features, patch, ready-to-commit, security
> Attachments: InvokeHTTP_2-way_SSL_support.patch
>
>
> HandleHTTPRequest processor as server, supports 2Way SSL( ie client
> authentication). InvokeHTTP processor as client, unfortunately not. I would
> like provide my patch for InvokeHTTP processor.See attach.
> Here some comments for code.
> I added Client.Auth methods
> I added hostname validator.
> Due to original code base and chosen HTTP client, I changed OkHttpClient
> reference to OkHttpClient.Builder for host validation handler. I have not way
> to support EL in properties and pass them to handler from setupto trigger via
> context.
> {code:java}
> AtomicReference okHttpClientBuilderAtomicReferenc{code}
> Most hard and long operations done before trigger, while scheduler starts,
> building client is relatively lightweight.
> Some comments about host validator, reasons to do this.
> My case is build RESTful services with 2-way SSL authentication by IP. Remote
> client can be a servers at same time as a clients, like mutual communication,
> but no domains, only IPs in green field. More over, clients can change
> dynamically their IP due to selected channel, LAN or Cellular, here is not
> way to provide SAN to certificate at configuration. Now you can provide
> dynamically via EL/param IP addresses to check hostname for client
> authentication.
>
> PS. It's not clear code, why processor build SSLContext in SSL Context
> Controlller, but not use it anyhow? This is strange and unclear, possibly,
> here we can reduce the code.
> PPS. It not clear, how to build tests for this case.
>
>
> Sincerely,
> Dmitry.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NIFI-7134) Enable JettyServer to automatically detect keystore changes and update
[
https://issues.apache.org/jira/browse/NIFI-7134?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034876#comment-17034876
]
Andy LoPresto commented on NIFI-7134:
-
Thanks for filing this, Patrick. I think there are a couple different pieces
here that should be split out into multiple sub-tasks:
1. Allowing triggered reloading of certificate material without an application
restart
2. A separate monitoring process (could be in `bootstrap`) which detects
changes to the keystore contents (would need keystore password, etc.)
As the keystore & truststore and their relative passwords are specified in the
{{nifi.properties}} file (often in encrypted form), we would need to be very
careful about changing to a _new_ keystore or rotating a password without
requiring a restart to ensure the canonical source of truth (the
{{nifi.properties}} file) is always accurate. I think the specific scenario we
could support easily is reloading the keystore when a new certificate is
provided (likely in the same alias, perhaps in a new alias if NIFI-1995 is
implemented) with the requirement that the file path and password have not
changed. For enhanced behavior, we may need to make additional decisions about
where those values come from and would be stored.
> Enable JettyServer to automatically detect keystore changes and update
> --
>
> Key: NIFI-7134
> URL: https://issues.apache.org/jira/browse/NIFI-7134
> Project: Apache NiFi
> Issue Type: New Feature
> Components: Core Framework, Security
>Affects Versions: 1.11.1
>Reporter: patrick white
>Priority: Minor
> Labels: jetty, keystore, restart, security, tls
>
> TLS/keystore credential change currently requires a service restart to
> update, [~alopresto] noted on 'users' that Jetty 9.3+ supports the ability to
> dynamically update credentials, and provided reference [1].
> Request enabling NiFi JettyServer to support detection and reload of its
> keystore when it changes, such as during credentials update or rotation, will
> link this request to epic [2].
> [1] https://github.com/eclipse/jetty.project/issues/918
> [2] https://issues.apache.org/jira/browse/NIFI-5458
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Updated] (NIFI-7134) Enable JettyServer to automatically detect keystore changes and update
[ https://issues.apache.org/jira/browse/NIFI-7134?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andy LoPresto updated NIFI-7134: Issue Type: New Feature (was: Improvement) > Enable JettyServer to automatically detect keystore changes and update > -- > > Key: NIFI-7134 > URL: https://issues.apache.org/jira/browse/NIFI-7134 > Project: Apache NiFi > Issue Type: New Feature > Components: Core Framework, Security >Affects Versions: 1.11.1 >Reporter: patrick white >Priority: Minor > Labels: jetty, keystore, restart, security, tls > > TLS/keystore credential change currently requires a service restart to > update, [~alopresto] noted on 'users' that Jetty 9.3+ supports the ability to > dynamically update credentials, and provided reference [1]. > Request enabling NiFi JettyServer to support detection and reload of its > keystore when it changes, such as during credentials update or rotation, will > link this request to epic [2]. > [1] https://github.com/eclipse/jetty.project/issues/918 > [2] https://issues.apache.org/jira/browse/NIFI-5458 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (NIFI-7134) Enable JettyServer to automatically detect keystore changes and update
[ https://issues.apache.org/jira/browse/NIFI-7134?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andy LoPresto updated NIFI-7134: Affects Version/s: 1.11.1 > Enable JettyServer to automatically detect keystore changes and update > -- > > Key: NIFI-7134 > URL: https://issues.apache.org/jira/browse/NIFI-7134 > Project: Apache NiFi > Issue Type: Improvement > Components: Core Framework, Security >Affects Versions: 1.11.1 >Reporter: patrick white >Priority: Minor > Labels: jetty, keystore, restart, security, tls > > TLS/keystore credential change currently requires a service restart to > update, [~alopresto] noted on 'users' that Jetty 9.3+ supports the ability to > dynamically update credentials, and provided reference [1]. > Request enabling NiFi JettyServer to support detection and reload of its > keystore when it changes, such as during credentials update or rotation, will > link this request to epic [2]. > [1] https://github.com/eclipse/jetty.project/issues/918 > [2] https://issues.apache.org/jira/browse/NIFI-5458 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (NIFI-7134) Enable JettyServer to automatically detect keystore changes and update
[ https://issues.apache.org/jira/browse/NIFI-7134?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andy LoPresto updated NIFI-7134: Labels: jetty keystore restart security tls (was: ) > Enable JettyServer to automatically detect keystore changes and update > -- > > Key: NIFI-7134 > URL: https://issues.apache.org/jira/browse/NIFI-7134 > Project: Apache NiFi > Issue Type: Improvement > Components: Core Framework, Security >Reporter: patrick white >Priority: Minor > Labels: jetty, keystore, restart, security, tls > > TLS/keystore credential change currently requires a service restart to > update, [~alopresto] noted on 'users' that Jetty 9.3+ supports the ability to > dynamically update credentials, and provided reference [1]. > Request enabling NiFi JettyServer to support detection and reload of its > keystore when it changes, such as during credentials update or rotation, will > link this request to epic [2]. > [1] https://github.com/eclipse/jetty.project/issues/918 > [2] https://issues.apache.org/jira/browse/NIFI-5458 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (NIFI-7134) Enable JettyServer to automatically detect keystore changes and update
[ https://issues.apache.org/jira/browse/NIFI-7134?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andy LoPresto updated NIFI-7134: Component/s: Security > Enable JettyServer to automatically detect keystore changes and update > -- > > Key: NIFI-7134 > URL: https://issues.apache.org/jira/browse/NIFI-7134 > Project: Apache NiFi > Issue Type: Improvement > Components: Core Framework, Security >Reporter: patrick white >Priority: Minor > > TLS/keystore credential change currently requires a service restart to > update, [~alopresto] noted on 'users' that Jetty 9.3+ supports the ability to > dynamically update credentials, and provided reference [1]. > Request enabling NiFi JettyServer to support detection and reload of its > keystore when it changes, such as during credentials update or rotation, will > link this request to epic [2]. > [1] https://github.com/eclipse/jetty.project/issues/918 > [2] https://issues.apache.org/jira/browse/NIFI-5458 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi-minifi-cpp] msharee9 opened a new pull request #734: MINIFICPP-1157 Implement light weight heartbeat.
msharee9 opened a new pull request #734: MINIFICPP-1157 Implement light weight heartbeat. URL: https://github.com/apache/nifi-minifi-cpp/pull/734 Remove agent manifest from regular heartbeat messages. Send agent manifest in response to DESCRIBE manifest request 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 With regards, Apache Git Services
[jira] [Created] (NIFI-7134) Enable JettyServer to automatically detect keystore changes and update
patrick white created NIFI-7134: --- Summary: Enable JettyServer to automatically detect keystore changes and update Key: NIFI-7134 URL: https://issues.apache.org/jira/browse/NIFI-7134 Project: Apache NiFi Issue Type: Improvement Components: Core Framework Reporter: patrick white TLS/keystore credential change currently requires a service restart to update, [~alopresto] noted on 'users' that Jetty 9.3+ supports the ability to dynamically update credentials, and provided reference [1]. Request enabling NiFi JettyServer to support detection and reload of its keystore when it changes, such as during credentials update or rotation, will link this request to epic [2]. [1] https://github.com/eclipse/jetty.project/issues/918 [2] https://issues.apache.org/jira/browse/NIFI-5458 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (MINIFICPP-1157) Implement lightweight C2 heartbeat
[ https://issues.apache.org/jira/browse/MINIFICPP-1157?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Murtuza Shareef updated MINIFICPP-1157: --- Description: Agent manifest doesn't need to be sent in every heartbeat message. C2 server can request the manifest in DESCRIBE operation. > Implement lightweight C2 heartbeat > -- > > Key: MINIFICPP-1157 > URL: https://issues.apache.org/jira/browse/MINIFICPP-1157 > Project: Apache NiFi MiNiFi C++ > Issue Type: Story >Reporter: Murtuza Shareef >Assignee: Murtuza Shareef >Priority: Major > > Agent manifest doesn't need to be sent in every heartbeat message. C2 server > can request the manifest in DESCRIBE operation. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (MINIFICPP-1157) Implement lightweight C2 heartbeat
Murtuza Shareef created MINIFICPP-1157: -- Summary: Implement lightweight C2 heartbeat Key: MINIFICPP-1157 URL: https://issues.apache.org/jira/browse/MINIFICPP-1157 Project: Apache NiFi MiNiFi C++ Issue Type: Story Reporter: Murtuza Shareef Assignee: Murtuza Shareef -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (NIFIREG-360) nifireg connecting to postgresql error
Carl Eastman created NIFIREG-360: Summary: nifireg connecting to postgresql error Key: NIFIREG-360 URL: https://issues.apache.org/jira/browse/NIFIREG-360 Project: NiFi Registry Issue Type: Bug Affects Versions: 0.5.0 Environment: IBM Cloud - Databases for postgresql Reporter: Carl Eastman snippet from the app.log: Caused by: org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'registryService' defined in URL [jar:file:/nifi-registry/work/jetty/nifi-registry-web-api-0.5.0.war/webapp/WEB-INF/lib/nifi-registry-framework-0.5.0.jar!/org/apache/nifi/registry/service/RegistryService.class]: Unsatisfied dependency expressed through constructor parameter 0; nested exception is org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'databaseMetadataService' defined in URL [jar:file:/nifi-registry/work/jetty/nifi-registry-web-api-0.5.0.war/webapp/WEB-INF/lib/nifi-registry-framework-0.5.0.jar!/org/apache/nifi/registry/db/DatabaseMetadataService.class]: Unsatisfied dependency expressed through constructor parameter 0; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'flywayInitializer' defined in class path resource [org/springframework/boot/autoconfigure/flyway/FlywayAutoConfiguration$FlywayConfiguration.class]: Invocation of init method failed; nested exception is org.flywaydb.core.api.FlywayException: Found non-empty schema(s) "public" without schema history table! Use baseline() or set baselineOnMigrate to true to initialize the schema history table. at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:769) at org.springframework.beans.factory.support.ConstructorResolver.autowireConstructor(ConstructorResolver.java:218) at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.autowireConstructor(AbstractAutowireCapableBeanFactory.java:1341) at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1187) at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:555) at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:515) at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:320) at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:222) at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:318) at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:199) at org.springframework.beans.factory.config.DependencyDescriptor.resolveCandidate(DependencyDescriptor.java:277) at org.springframework.beans.factory.support.DefaultListableBeanFactory.doResolveDependency(DefaultListableBeanFactory.java:1251) at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveDependency(DefaultListableBeanFactory.java:1171) at org.springframework.beans.factory.support.ConstructorResolver.resolveAutowiredArgument(ConstructorResolver.java:857) at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:760) ... 66 common frames omitted -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] CZuegner commented on issue #4035: NIFI-7097: ResultSetRecordSet: Always use RecordField from readerSchema if applicable.
CZuegner commented on issue #4035: NIFI-7097: ResultSetRecordSet: Always use RecordField from readerSchema if applicable. URL: https://github.com/apache/nifi/pull/4035#issuecomment-584831086 I've added a Unit Test for ResultSetRecordSet with and without a given schema. With a given schema it is possible to extract the correct Data Type from Arrays instead of fallback to String Type when schema is inferred. 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 With regards, Apache Git Services
[jira] [Resolved] (NIFI-6873) Add support to import versioned flow
[ https://issues.apache.org/jira/browse/NIFI-6873?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Mark Payne resolved NIFI-6873. -- Fix Version/s: 1.12.0 Resolution: Fixed > Add support to import versioned flow > > > Key: NIFI-6873 > URL: https://issues.apache.org/jira/browse/NIFI-6873 > Project: Apache NiFi > Issue Type: Task > Components: Core Framework, Core UI >Reporter: Matt Gilman >Assignee: Joe Ferner >Priority: Major > Fix For: 1.12.0 > > Time Spent: 2h > Remaining Estimate: 0h > > When integrating with a NiFi Registry, NiFi can generate versioned flows to > store in the registry. We should also allow these versioned flows to be > imported directly. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] markap14 commented on issue #4023: NIFI-6873: Added support for replacing a process group via import
markap14 commented on issue #4023: NIFI-6873: Added support for replacing a process group via import URL: https://github.com/apache/nifi/pull/4023#issuecomment-584818170 Thanks @jsferner - this PR is really well put together! Code is well written and easy to follow. The additional commits in the PR made it really easy to understand how to test and verify behavior. +1 merged to master 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 With regards, Apache Git Services
[jira] [Commented] (NIFI-6873) Add support to import versioned flow
[ https://issues.apache.org/jira/browse/NIFI-6873?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034773#comment-17034773 ] ASF subversion and git services commented on NIFI-6873: --- Commit 62606ff89a5947197fc0a92adae9e7a5104ca2b7 in nifi's branch refs/heads/master from Joe Ferner [ https://gitbox.apache.org/repos/asf?p=nifi.git;h=62606ff ] NIFI-6873: Added support for replacing a process group - decoupled flow update request behavior from VersionsResource into new abstract FlowUpdateResource - added replace process group functionality in ProcessGroupResource - parameterized FlowUpdateResource and created entity hierarchies to allow for maximum code sharing across different update types - refactored flow update methods to make use of commonality across different update types whenever possible - fixed issues in StandardProcessGroup verify update methods where same components existed in different ancestry chains but were considered a match when they shouldn't be - improved StandardProcessGroup to properly match up components on update using generated versioned component ids, when necessary to allow for update flow to efficiently match common components on flow import This closes #4023. Signed-off-by: Mark Payne > Add support to import versioned flow > > > Key: NIFI-6873 > URL: https://issues.apache.org/jira/browse/NIFI-6873 > Project: Apache NiFi > Issue Type: Task > Components: Core Framework, Core UI >Reporter: Matt Gilman >Assignee: Joe Ferner >Priority: Major > Time Spent: 1h 40m > Remaining Estimate: 0h > > When integrating with a NiFi Registry, NiFi can generate versioned flows to > store in the registry. We should also allow these versioned flows to be > imported directly. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] asfgit closed pull request #4023: NIFI-6873: Added support for replacing a process group via import
asfgit closed pull request #4023: NIFI-6873: Added support for replacing a process group via import URL: https://github.com/apache/nifi/pull/4023 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 With regards, Apache Git Services
[jira] [Resolved] (NIFI-7132) PutCassandraQL is handling UUIDs as Strings
[ https://issues.apache.org/jira/browse/NIFI-7132?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Pierre Villard resolved NIFI-7132. -- Fix Version/s: 1.12.0 Resolution: Fixed > PutCassandraQL is handling UUIDs as Strings > --- > > Key: NIFI-7132 > URL: https://issues.apache.org/jira/browse/NIFI-7132 > Project: Apache NiFi > Issue Type: Bug > Components: Extensions >Affects Versions: 1.11.1 >Reporter: Wouter de Vries >Priority: Major > Labels: cassandra, putcassandraql, uuid > Fix For: 1.12.0 > > Time Spent: 20m > Remaining Estimate: 0h > > PutCassandraQL fails when attempting to insert a UUID/timeUUID on Nifi 1.11.1 > (and presumably all prior versions), due to handling UUIDs as strings. > Input: > FlowFile Attribute Map Content > Key: 'cql.args.1.type' > Value: 'timeuuid' > Key: 'cql.args.1.value' > Value: '5442b1f6-4c16-11ea-87f5-45a32dbc5199' > -- > INSERT INTO test.test (id) VALUES ( ? ) > > Output: > com.datastax.driver.core.exceptions.CodecNotFoundException: Codec not found > for requested operation: [timeuuid <-> java.lang.String] > Record does not get inserted into Cassandra > Expected output: > Record gets inserted into Cassandra -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] pvillard31 commented on issue #4048: NIFI-7132: fix handling of UUIDs in PutCassandraQL
pvillard31 commented on issue #4048: NIFI-7132: fix handling of UUIDs in PutCassandraQL URL: https://github.com/apache/nifi/pull/4048#issuecomment-584806564 LGTM, +1, merged to master, thanks @woutifier-t 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 With regards, Apache Git Services
[GitHub] [nifi] asfgit closed pull request #4048: NIFI-7132: fix handling of UUIDs in PutCassandraQL
asfgit closed pull request #4048: NIFI-7132: fix handling of UUIDs in PutCassandraQL URL: https://github.com/apache/nifi/pull/4048 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 With regards, Apache Git Services
[jira] [Commented] (NIFI-7132) PutCassandraQL is handling UUIDs as Strings
[ https://issues.apache.org/jira/browse/NIFI-7132?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034756#comment-17034756 ] ASF subversion and git services commented on NIFI-7132: --- Commit a80b2475f79ae8142202e64bb7822f05814b2dd4 in nifi's branch refs/heads/master from Wouter de Vries [ https://gitbox.apache.org/repos/asf?p=nifi.git;h=a80b247 ] NIFI-7132: fix handling of UUIDs in PutCassandraQL Signed-off-by: Pierre Villard This closes #4048. > PutCassandraQL is handling UUIDs as Strings > --- > > Key: NIFI-7132 > URL: https://issues.apache.org/jira/browse/NIFI-7132 > Project: Apache NiFi > Issue Type: Bug > Components: Extensions >Affects Versions: 1.11.1 >Reporter: Wouter de Vries >Priority: Major > Labels: cassandra, putcassandraql, uuid > Time Spent: 10m > Remaining Estimate: 0h > > PutCassandraQL fails when attempting to insert a UUID/timeUUID on Nifi 1.11.1 > (and presumably all prior versions), due to handling UUIDs as strings. > Input: > FlowFile Attribute Map Content > Key: 'cql.args.1.type' > Value: 'timeuuid' > Key: 'cql.args.1.value' > Value: '5442b1f6-4c16-11ea-87f5-45a32dbc5199' > -- > INSERT INTO test.test (id) VALUES ( ? ) > > Output: > com.datastax.driver.core.exceptions.CodecNotFoundException: Codec not found > for requested operation: [timeuuid <-> java.lang.String] > Record does not get inserted into Cassandra > Expected output: > Record gets inserted into Cassandra -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NIFI-7064) Support 2-way SSL by InvokeHTTP processor
[
https://issues.apache.org/jira/browse/NIFI-7064?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034757#comment-17034757
]
Dmitry Mashkov commented on NIFI-7064:
--
Hi Shawn, Andy,
Sorry for a little bit delay with answers. Now I'm doubted too. I will revert
back and retest my case and I'll be back with you shortly.
Some questions from me, why SSLContext was built in
StandardSSLContextService and rejected in {{InvokeHTTP?}}
> Support 2-way SSL by InvokeHTTP processor
> -
>
> Key: NIFI-7064
> URL: https://issues.apache.org/jira/browse/NIFI-7064
> Project: Apache NiFi
> Issue Type: Improvement
> Components: Core Framework, Security
>Affects Versions: 1.10.0
>Reporter: Dmitry Mashkov
>Priority: Major
> Labels: features, patch, ready-to-commit, security
> Attachments: InvokeHTTP_2-way_SSL_support.patch
>
>
> HandleHTTPRequest processor as server, supports 2Way SSL( ie client
> authentication). InvokeHTTP processor as client, unfortunately not. I would
> like provide my patch for InvokeHTTP processor.See attach.
> Here some comments for code.
> I added Client.Auth methods
> I added hostname validator.
> Due to original code base and chosen HTTP client, I changed OkHttpClient
> reference to OkHttpClient.Builder for host validation handler. I have not way
> to support EL in properties and pass them to handler from setupto trigger via
> context.
> {code:java}
> AtomicReference okHttpClientBuilderAtomicReferenc{code}
> Most hard and long operations done before trigger, while scheduler starts,
> building client is relatively lightweight.
> Some comments about host validator, reasons to do this.
> My case is build RESTful services with 2-way SSL authentication by IP. Remote
> client can be a servers at same time as a clients, like mutual communication,
> but no domains, only IPs in green field. More over, clients can change
> dynamically their IP due to selected channel, LAN or Cellular, here is not
> way to provide SAN to certificate at configuration. Now you can provide
> dynamically via EL/param IP addresses to check hostname for client
> authentication.
>
> PS. It's not clear code, why processor build SSLContext in SSL Context
> Controlller, but not use it anyhow? This is strange and unclear, possibly,
> here we can reduce the code.
> PPS. It not clear, how to build tests for this case.
>
>
> Sincerely,
> Dmitry.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [nifi] pvillard31 opened a new pull request #4049: NIFI-7133 - Clarification of EnforceOrder description
pvillard31 opened a new pull request #4049: NIFI-7133 - Clarification of
EnforceOrder description
URL: https://github.com/apache/nifi/pull/4049
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 both JDK 8 and
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 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
With regards,
Apache Git Services
[jira] [Updated] (NIFI-7133) Clarification of EnforceOrder description
[ https://issues.apache.org/jira/browse/NIFI-7133?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Pierre Villard updated NIFI-7133: - Assignee: Pierre Villard Status: Patch Available (was: Open) > Clarification of EnforceOrder description > - > > Key: NIFI-7133 > URL: https://issues.apache.org/jira/browse/NIFI-7133 > Project: Apache NiFi > Issue Type: Improvement > Components: Documentation Website >Reporter: Dennis Jaheruddin >Assignee: Pierre Villard >Priority: Trivial > Time Spent: 10m > Remaining Estimate: 0h > > The EnforceOrder processor only enforces order within a single node. Though > the description has some disclaimers, I would request this to be mentioned > explicitly. For instance: > > Enforces expected ordering of FlowFiles that belong to the same data group > within a single node. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NIFI-7123) onPropertyModified() is called on nifi start up even when properties have never been modified
[
https://issues.apache.org/jira/browse/NIFI-7123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034740#comment-17034740
]
Nissim Shiman commented on NIFI-7123:
-
If this is considered an issue, it won't be big deal to fix (i.e. minimal
touching of core code)
It will just need the following around the onPropertyModified() [1] :
if (!(descriptor.getDefaultValue().equals(effectiveValue) && oldValue ==
null)) {}
[1]https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/controller/AbstractComponentNode.java#L366
> onPropertyModified() is called on nifi start up even when properties have
> never been modified
> -
>
> Key: NIFI-7123
> URL: https://issues.apache.org/jira/browse/NIFI-7123
> Project: Apache NiFi
> Issue Type: Bug
>Affects Versions: 1.11.0
>Reporter: Nissim Shiman
>Assignee: Nissim Shiman
>Priority: Major
>
> Processors and Controller Services inherit the onPropertyModified() method
> from ConfigurableComponent. java [1]
> This method is called when nifi starts for all processors and controller
> services, even for properties that are set to their defaults (i.e. have never
> been modified).
> [1]
> https://github.com/apache/nifi/blob/master/nifi-api/src/main/java/org/apache/nifi/components/ConfigurableComponent.java#L68
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Updated] (NIFI-7117) Load Balance is not working
[ https://issues.apache.org/jira/browse/NIFI-7117?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Pierre Villard updated NIFI-7117: - Resolution: Fixed Status: Resolved (was: Patch Available) > Load Balance is not working > --- > > Key: NIFI-7117 > URL: https://issues.apache.org/jira/browse/NIFI-7117 > Project: Apache NiFi > Issue Type: Bug > Components: Core Framework >Affects Versions: 1.11.1 >Reporter: Eduardo Mota Fontes >Assignee: Mark Payne >Priority: Blocker > Fix For: 1.12.0, 1.11.2 > > Time Spent: 2h 10m > Remaining Estimate: 0h > > Load Balancing is not working on 1.11.1. > I've tested with a 3 node cluster, GenerateFlowFile on All Nodes and a > connection with Single Node Load Balancing. All FlowFiles remains in its node. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] pvillard31 commented on issue #4045: NIFI-7117: When SocketLoadBalancedFlowFileQueue creates its array of …
pvillard31 commented on issue #4045: NIFI-7117: When SocketLoadBalancedFlowFileQueue creates its array of … URL: https://github.com/apache/nifi/pull/4045#issuecomment-584797494 Merged into master, thanks @markap14 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 With regards, Apache Git Services
[jira] [Commented] (NIFI-7117) Load Balance is not working
[ https://issues.apache.org/jira/browse/NIFI-7117?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034738#comment-17034738 ] ASF subversion and git services commented on NIFI-7117: --- Commit 65b2a9bc2c91657bc160fe93d6051da95ab5db34 in nifi's branch refs/heads/master from Mark Payne [ https://gitbox.apache.org/repos/asf?p=nifi.git;h=65b2a9b ] NIFI-7117: When SocketLoadBalancedFlowFileQueue creates its array of Queue Partitions in the constructor, it added the local partition as the first element in that list. This list should be ordered the same across all nodes in the cluster. By making the local partition the first in the array, each node had a different ordering of these partitions. As a result, Partition by Attribute strategy would constantly rebalance flowfiles that it received to other node, and Single Node always transferred data to the first partition, which was the local node, instead of whichever node should have been the first in the list. This commit addresses this issue by instead inserting the local partition intot he 'queuePartitions' array based on the local node identifier. Signed-off-by: Pierre Villard This closes #4045. > Load Balance is not working > --- > > Key: NIFI-7117 > URL: https://issues.apache.org/jira/browse/NIFI-7117 > Project: Apache NiFi > Issue Type: Bug > Components: Core Framework >Affects Versions: 1.11.1 >Reporter: Eduardo Mota Fontes >Assignee: Mark Payne >Priority: Blocker > Fix For: 1.12.0, 1.11.2 > > Time Spent: 2h > Remaining Estimate: 0h > > Load Balancing is not working on 1.11.1. > I've tested with a 3 node cluster, GenerateFlowFile on All Nodes and a > connection with Single Node Load Balancing. All FlowFiles remains in its node. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] asfgit closed pull request #4045: NIFI-7117: When SocketLoadBalancedFlowFileQueue creates its array of …
asfgit closed pull request #4045: NIFI-7117: When SocketLoadBalancedFlowFileQueue creates its array of … URL: https://github.com/apache/nifi/pull/4045 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 With regards, Apache Git Services
[GitHub] [nifi] markap14 commented on a change in pull request #4045: NIFI-7117: When SocketLoadBalancedFlowFileQueue creates its array of …
markap14 commented on a change in pull request #4045: NIFI-7117: When
SocketLoadBalancedFlowFileQueue creates its array of …
URL: https://github.com/apache/nifi/pull/4045#discussion_r377833917
##
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/queue/clustered/SocketLoadBalancedFlowFileQueue.java
##
@@ -151,15 +151,19 @@ public SocketLoadBalancedFlowFileQueue(final String
identifier, final Connection
// that is not the local node identifier. If the Local Node
Identifier is not yet known, that's okay. When it becomes known,
// the queuePartitions array will be recreated with the
appropriate partitions.
final List partitionList = new ArrayList<>();
-partitionList.add(localPartition);
final NodeIdentifier localNodeId =
clusterCoordinator.getLocalNodeIdentifier();
for (final NodeIdentifier nodeId : sortedNodeIdentifiers) {
if (nodeId.equals(localNodeId)) {
-continue;
+partitionList.add(localPartition);
+} else {
+partitionList.add(createRemotePartition(nodeId));
}
+}
-partitionList.add(createRemotePartition(nodeId));
+// Ensure that our list of queue partitions always contains the
local partition.
+if (!partitionList.contains(localPartition)) {
Review comment:
No problem, glad I was able to clarify.
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
With regards,
Apache Git Services
[GitHub] [nifi] eduardofontes commented on issue #4045: NIFI-7117: When SocketLoadBalancedFlowFileQueue creates its array of …
eduardofontes commented on issue #4045: NIFI-7117: When SocketLoadBalancedFlowFileQueue creates its array of … URL: https://github.com/apache/nifi/pull/4045#issuecomment-584783372 PR built and tested with Single Node in a 3 node cluster. +1 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [nifi] ivan1221 commented on a change in pull request #4045: NIFI-7117: When SocketLoadBalancedFlowFileQueue creates its array of …
ivan1221 commented on a change in pull request #4045: NIFI-7117: When
SocketLoadBalancedFlowFileQueue creates its array of …
URL: https://github.com/apache/nifi/pull/4045#discussion_r377812936
##
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/queue/clustered/SocketLoadBalancedFlowFileQueue.java
##
@@ -151,15 +151,19 @@ public SocketLoadBalancedFlowFileQueue(final String
identifier, final Connection
// that is not the local node identifier. If the Local Node
Identifier is not yet known, that's okay. When it becomes known,
// the queuePartitions array will be recreated with the
appropriate partitions.
final List partitionList = new ArrayList<>();
-partitionList.add(localPartition);
final NodeIdentifier localNodeId =
clusterCoordinator.getLocalNodeIdentifier();
for (final NodeIdentifier nodeId : sortedNodeIdentifiers) {
if (nodeId.equals(localNodeId)) {
-continue;
+partitionList.add(localPartition);
+} else {
+partitionList.add(createRemotePartition(nodeId));
}
+}
-partitionList.add(createRemotePartition(nodeId));
+// Ensure that our list of queue partitions always contains the
local partition.
Review comment:
all clear.
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
With regards,
Apache Git Services
[GitHub] [nifi] ivan1221 commented on a change in pull request #4045: NIFI-7117: When SocketLoadBalancedFlowFileQueue creates its array of …
ivan1221 commented on a change in pull request #4045: NIFI-7117: When
SocketLoadBalancedFlowFileQueue creates its array of …
URL: https://github.com/apache/nifi/pull/4045#discussion_r377812741
##
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/queue/clustered/SocketLoadBalancedFlowFileQueue.java
##
@@ -151,15 +151,19 @@ public SocketLoadBalancedFlowFileQueue(final String
identifier, final Connection
// that is not the local node identifier. If the Local Node
Identifier is not yet known, that's okay. When it becomes known,
// the queuePartitions array will be recreated with the
appropriate partitions.
final List partitionList = new ArrayList<>();
-partitionList.add(localPartition);
final NodeIdentifier localNodeId =
clusterCoordinator.getLocalNodeIdentifier();
for (final NodeIdentifier nodeId : sortedNodeIdentifiers) {
if (nodeId.equals(localNodeId)) {
-continue;
+partitionList.add(localPartition);
+} else {
+partitionList.add(createRemotePartition(nodeId));
}
+}
-partitionList.add(createRemotePartition(nodeId));
+// Ensure that our list of queue partitions always contains the
local partition.
+if (!partitionList.contains(localPartition)) {
Review comment:
Perfect, I understand. Thank you very much for your explanation. I am very
interested in this commit to be able to take the version to production.**+1**
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi] markap14 commented on a change in pull request #4045: NIFI-7117: When SocketLoadBalancedFlowFileQueue creates its array of …
markap14 commented on a change in pull request #4045: NIFI-7117: When
SocketLoadBalancedFlowFileQueue creates its array of …
URL: https://github.com/apache/nifi/pull/4045#discussion_r377809799
##
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/queue/clustered/SocketLoadBalancedFlowFileQueue.java
##
@@ -151,15 +151,19 @@ public SocketLoadBalancedFlowFileQueue(final String
identifier, final Connection
// that is not the local node identifier. If the Local Node
Identifier is not yet known, that's okay. When it becomes known,
// the queuePartitions array will be recreated with the
appropriate partitions.
final List partitionList = new ArrayList<>();
-partitionList.add(localPartition);
final NodeIdentifier localNodeId =
clusterCoordinator.getLocalNodeIdentifier();
for (final NodeIdentifier nodeId : sortedNodeIdentifiers) {
if (nodeId.equals(localNodeId)) {
-continue;
+partitionList.add(localPartition);
+} else {
+partitionList.add(createRemotePartition(nodeId));
}
+}
-partitionList.add(createRemotePartition(nodeId));
+// Ensure that our list of queue partitions always contains the
local partition.
+if (!partitionList.contains(localPartition)) {
Review comment:
You're correct. If local node id ends up being null, we will add
localPartition to the end of this, which means that this node will have the
order wrong. As the code comment above explains, though, that's okay. The order
will only be wrong until the local node identifier is determined. This will
generally take milliseconds. Perhaps a few seconds. Then the ordering will be
fixed in the `onLocalNodeIdentifierSet` method. In the meantime, it may end up
sending the data to the wrong node, and the receiving node would then take that
data and re-send it to who it believes to be the correct node in the cluster.
It is inefficient, but should only occur for a very short period of time and
only when the local node id is null (and this happens rarely, as it's basically
a race condition that can happen when the node is added to the cluster for the
first time. The next time the cluster is restarted, the local node id should be
known from local state.)
The problem previously was that the localPartition was *always* added as the
first entry, even when the local node identifier was already known. As a
result, it didn't get re-sorted. Now, it will be added only when the local nod
id is null and therefore it will be re-sorted when the local node id is known.
Sorry for the huge explanation. I hope it all makes sense.
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
With regards,
Apache Git Services
[jira] [Commented] (NIFI-7117) Load Balance is not working
[ https://issues.apache.org/jira/browse/NIFI-7117?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034679#comment-17034679 ] Eduardo Mota Fontes commented on NIFI-7117: --- Hi! PR [4045|https://github.com/apache/nifi/pull/4045] compiled and tested with 3 node cluster. Now Single Node Load Balancing puts all flowfiles in one node. Thank you [~markap14] and [~joewitt] > Load Balance is not working > --- > > Key: NIFI-7117 > URL: https://issues.apache.org/jira/browse/NIFI-7117 > Project: Apache NiFi > Issue Type: Bug > Components: Core Framework >Affects Versions: 1.11.1 >Reporter: Eduardo Mota Fontes >Assignee: Mark Payne >Priority: Blocker > Fix For: 1.12.0, 1.11.2 > > Time Spent: 1h 10m > Remaining Estimate: 0h > > Load Balancing is not working on 1.11.1. > I've tested with a 3 node cluster, GenerateFlowFile on All Nodes and a > connection with Single Node Load Balancing. All FlowFiles remains in its node. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] markap14 commented on a change in pull request #4045: NIFI-7117: When SocketLoadBalancedFlowFileQueue creates its array of …
markap14 commented on a change in pull request #4045: NIFI-7117: When
SocketLoadBalancedFlowFileQueue creates its array of …
URL: https://github.com/apache/nifi/pull/4045#discussion_r377806864
##
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/queue/clustered/SocketLoadBalancedFlowFileQueue.java
##
@@ -151,15 +151,19 @@ public SocketLoadBalancedFlowFileQueue(final String
identifier, final Connection
// that is not the local node identifier. If the Local Node
Identifier is not yet known, that's okay. When it becomes known,
// the queuePartitions array will be recreated with the
appropriate partitions.
final List partitionList = new ArrayList<>();
-partitionList.add(localPartition);
final NodeIdentifier localNodeId =
clusterCoordinator.getLocalNodeIdentifier();
for (final NodeIdentifier nodeId : sortedNodeIdentifiers) {
if (nodeId.equals(localNodeId)) {
-continue;
+partitionList.add(localPartition);
+} else {
+partitionList.add(createRemotePartition(nodeId));
}
+}
-partitionList.add(createRemotePartition(nodeId));
+// Ensure that our list of queue partitions always contains the
local partition.
Review comment:
Yes, `clusterCoordinator.getLocalNodeIdentifier()` can return `null`. In
that case, the local partition will never be added in the loop.
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
With regards,
Apache Git Services
[GitHub] [nifi] ottobackwards commented on a change in pull request #4043: NIFI-7109 Unit tests should be able to determine if item validator wa…
ottobackwards commented on a change in pull request #4043: NIFI-7109 Unit tests should be able to determine if item validator wa… URL: https://github.com/apache/nifi/pull/4043#discussion_r377803491 ## File path: nifi-mock/src/test/java/org/apache/nifi/util/validator/InstrumentedValidator.java ## @@ -0,0 +1,92 @@ +/* Review comment: ok, working on it 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 With regards, Apache Git Services
[GitHub] [nifi] ivan1221 commented on a change in pull request #4045: NIFI-7117: When SocketLoadBalancedFlowFileQueue creates its array of …
ivan1221 commented on a change in pull request #4045: NIFI-7117: When
SocketLoadBalancedFlowFileQueue creates its array of …
URL: https://github.com/apache/nifi/pull/4045#discussion_r377791523
##
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/queue/clustered/SocketLoadBalancedFlowFileQueue.java
##
@@ -151,15 +151,19 @@ public SocketLoadBalancedFlowFileQueue(final String
identifier, final Connection
// that is not the local node identifier. If the Local Node
Identifier is not yet known, that's okay. When it becomes known,
// the queuePartitions array will be recreated with the
appropriate partitions.
final List partitionList = new ArrayList<>();
-partitionList.add(localPartition);
final NodeIdentifier localNodeId =
clusterCoordinator.getLocalNodeIdentifier();
for (final NodeIdentifier nodeId : sortedNodeIdentifiers) {
if (nodeId.equals(localNodeId)) {
-continue;
+partitionList.add(localPartition);
+} else {
+partitionList.add(createRemotePartition(nodeId));
}
+}
-partitionList.add(createRemotePartition(nodeId));
+// Ensure that our list of queue partitions always contains the
local partition.
+if (!partitionList.contains(localPartition)) {
Review comment:
on the other hand, the order would not be altered if it happens that the
partition is not in the list for one of the nodes?
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
With regards,
Apache Git Services
[GitHub] [nifi] ivan1221 commented on a change in pull request #4045: NIFI-7117: When SocketLoadBalancedFlowFileQueue creates its array of …
ivan1221 commented on a change in pull request #4045: NIFI-7117: When
SocketLoadBalancedFlowFileQueue creates its array of …
URL: https://github.com/apache/nifi/pull/4045#discussion_r377732154
##
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/queue/clustered/SocketLoadBalancedFlowFileQueue.java
##
@@ -151,15 +151,19 @@ public SocketLoadBalancedFlowFileQueue(final String
identifier, final Connection
// that is not the local node identifier. If the Local Node
Identifier is not yet known, that's okay. When it becomes known,
// the queuePartitions array will be recreated with the
appropriate partitions.
final List partitionList = new ArrayList<>();
-partitionList.add(localPartition);
final NodeIdentifier localNodeId =
clusterCoordinator.getLocalNodeIdentifier();
for (final NodeIdentifier nodeId : sortedNodeIdentifiers) {
if (nodeId.equals(localNodeId)) {
-continue;
+partitionList.add(localPartition);
+} else {
+partitionList.add(createRemotePartition(nodeId));
}
+}
-partitionList.add(createRemotePartition(nodeId));
+// Ensure that our list of queue partitions always contains the
local partition.
Review comment:
on the other hand, the order would not be altered if it happens that the
partition is not in the list for one of the nodes?
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
With regards,
Apache Git Services
[jira] [Commented] (NIFI-4890) OIDC Token Refresh is not done correctly
[ https://issues.apache.org/jira/browse/NIFI-4890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034645#comment-17034645 ] N commented on NIFI-4890: - [~mcgilman] No no, what i meant was that nifi honors the expiration date and i wondered if there is a workaround around that. And about the refresh token im a bit of a novice so i cant give a specific answer, however i did receive an example from my IdP, which includes a new expiration date with some other fields that i include if the new expiration date is not enough for you > OIDC Token Refresh is not done correctly > > > Key: NIFI-4890 > URL: https://issues.apache.org/jira/browse/NIFI-4890 > Project: Apache NiFi > Issue Type: Bug > Components: Core UI >Affects Versions: 1.5.0 > Environment: Environment: > Browser: Chrome / Firefox > Configuration of NiFi: > - SSL certificate for the server (no client auth) > - OIDC configuration including end_session_endpoint (see the link > https://auth.s.orchestracities.com/auth/realms/default/.well-known/openid-configuration) > >Reporter: Federico Michele Facca >Priority: Major > > It looks like the NIFI UI is not refreshing the OIDC token in background, and > because of that, when the token expires, tells you that your session is > expired. and you need to refresh the page, to get a new token. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] mattyb149 commented on a change in pull request #4043: NIFI-7109 Unit tests should be able to determine if item validator wa…
mattyb149 commented on a change in pull request #4043: NIFI-7109 Unit tests should be able to determine if item validator wa… URL: https://github.com/apache/nifi/pull/4043#discussion_r377783726 ## File path: nifi-mock/src/test/java/org/apache/nifi/util/validator/InstrumentedValidator.java ## @@ -0,0 +1,92 @@ +/* Review comment: This one should go in the main sources so it's available to other modules to test their validators, otherwise it is only available to test classes in this module 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 With regards, Apache Git Services
[jira] [Updated] (NIFI-4792) Allow QueryRecord processor to query nested arrays
[ https://issues.apache.org/jira/browse/NIFI-4792?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Matt Burgess updated NIFI-4792: --- Fix Version/s: 1.12.0 Resolution: Fixed Status: Resolved (was: Patch Available) > Allow QueryRecord processor to query nested arrays > -- > > Key: NIFI-4792 > URL: https://issues.apache.org/jira/browse/NIFI-4792 > Project: Apache NiFi > Issue Type: Bug >Reporter: Julian Hyde >Priority: Major > Fix For: 1.12.0 > > Time Spent: 0.5h > Remaining Estimate: 0h > > Allow QueryRecord processor to query nested arrays. > See [email > thread|https://lists.apache.org/thread.html/5e9a0f18c132851833c5cde01301ec4aeb58dcfa7760ecc5b33179ba@%3Cusers.nifi.apache.org%3E]. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NIFI-4792) Allow QueryRecord processor to query nested arrays
[ https://issues.apache.org/jira/browse/NIFI-4792?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034578#comment-17034578 ] ASF subversion and git services commented on NIFI-4792: --- Commit 5e964fbc474034cb5342e22aa290f1040af377ea in nifi's branch refs/heads/master from David Savage [ https://gitbox.apache.org/repos/asf?p=nifi.git;h=5e964fb ] NIFI-4792: Add support for querying array fields in QueryRecord Work in progress adding support for array based queries updated calcite dependency tidy up unused imports highlighted by checkstyle in travis build tidy up }s highlighted by checkstyle in travis build Add test for use case referenced in NIFI-4792 Bumped Calcite version to 1.21.0 Signed-off-by: Matthew Burgess This closes #4015 > Allow QueryRecord processor to query nested arrays > -- > > Key: NIFI-4792 > URL: https://issues.apache.org/jira/browse/NIFI-4792 > Project: Apache NiFi > Issue Type: Bug >Reporter: Julian Hyde >Priority: Major > Time Spent: 20m > Remaining Estimate: 0h > > Allow QueryRecord processor to query nested arrays. > See [email > thread|https://lists.apache.org/thread.html/5e9a0f18c132851833c5cde01301ec4aeb58dcfa7760ecc5b33179ba@%3Cusers.nifi.apache.org%3E]. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NIFI-4792) Allow QueryRecord processor to query nested arrays
[ https://issues.apache.org/jira/browse/NIFI-4792?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034579#comment-17034579 ] ASF subversion and git services commented on NIFI-4792: --- Commit 5e964fbc474034cb5342e22aa290f1040af377ea in nifi's branch refs/heads/master from David Savage [ https://gitbox.apache.org/repos/asf?p=nifi.git;h=5e964fb ] NIFI-4792: Add support for querying array fields in QueryRecord Work in progress adding support for array based queries updated calcite dependency tidy up unused imports highlighted by checkstyle in travis build tidy up }s highlighted by checkstyle in travis build Add test for use case referenced in NIFI-4792 Bumped Calcite version to 1.21.0 Signed-off-by: Matthew Burgess This closes #4015 > Allow QueryRecord processor to query nested arrays > -- > > Key: NIFI-4792 > URL: https://issues.apache.org/jira/browse/NIFI-4792 > Project: Apache NiFi > Issue Type: Bug >Reporter: Julian Hyde >Priority: Major > Time Spent: 20m > Remaining Estimate: 0h > > Allow QueryRecord processor to query nested arrays. > See [email > thread|https://lists.apache.org/thread.html/5e9a0f18c132851833c5cde01301ec4aeb58dcfa7760ecc5b33179ba@%3Cusers.nifi.apache.org%3E]. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi] asfgit closed pull request #4015: NIFI-4792 Enable Array functions in QueryRecord
asfgit closed pull request #4015: NIFI-4792 Enable Array functions in QueryRecord URL: https://github.com/apache/nifi/pull/4015 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 With regards, Apache Git Services
[GitHub] [nifi] mattyb149 commented on issue #4015: NIFI-4792 Enable Array functions in QueryRecord
mattyb149 commented on issue #4015: NIFI-4792 Enable Array functions in QueryRecord URL: https://github.com/apache/nifi/pull/4015#issuecomment-584718090 +1 LGTM, ran contrib-check and on a live NiFi instance after bumping the Calcite version to 1.21. Everything ran well, thanks for the improvement! Merging to master 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 With regards, Apache Git Services
[GitHub] [nifi] ivan1221 commented on a change in pull request #4045: NIFI-7117: When SocketLoadBalancedFlowFileQueue creates its array of …
ivan1221 commented on a change in pull request #4045: NIFI-7117: When
SocketLoadBalancedFlowFileQueue creates its array of …
URL: https://github.com/apache/nifi/pull/4045#discussion_r377732154
##
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/queue/clustered/SocketLoadBalancedFlowFileQueue.java
##
@@ -151,15 +151,19 @@ public SocketLoadBalancedFlowFileQueue(final String
identifier, final Connection
// that is not the local node identifier. If the Local Node
Identifier is not yet known, that's okay. When it becomes known,
// the queuePartitions array will be recreated with the
appropriate partitions.
final List partitionList = new ArrayList<>();
-partitionList.add(localPartition);
final NodeIdentifier localNodeId =
clusterCoordinator.getLocalNodeIdentifier();
for (final NodeIdentifier nodeId : sortedNodeIdentifiers) {
if (nodeId.equals(localNodeId)) {
-continue;
+partitionList.add(localPartition);
+} else {
+partitionList.add(createRemotePartition(nodeId));
}
+}
-partitionList.add(createRemotePartition(nodeId));
+// Ensure that our list of queue partitions always contains the
local partition.
Review comment:
on the other hand, the order would not be altered if it happens that the
partition is not in the list for one of the nodes?
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
With regards,
Apache Git Services
[jira] [Commented] (NIFI-7114) NiFi not closing file handles
[ https://issues.apache.org/jira/browse/NIFI-7114?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034566#comment-17034566 ] Vinicius Zolin commented on NIFI-7114: -- [^lsof.zip] I've attached a zip with lsof output from yesterday to about now. Also, by plotting the number of files [^openFiles.xlsx]) you guys can see the behavior that I alluded to earlier: The leak takes circa 2 hours to start. > NiFi not closing file handles > - > > Key: NIFI-7114 > URL: https://issues.apache.org/jira/browse/NIFI-7114 > Project: Apache NiFi > Issue Type: Bug > Components: Configuration >Affects Versions: 1.10.0, 1.11.0 > Environment: Amazon EC2 running either Amazon Linux 2 or Ubuntu 18.04. > NiFi has been installed with no change to any configuration file. >Reporter: Vinicius Zolin >Priority: Major > Fix For: 1.11.2 > > Attachments: destination.xml, lsof.log, lsof.zip, lsofAfter.log, > lsofBefore.log, openFiles.xlsx, source.xml > > > Since at least version 1.10 NiFi stopped closing file handles. It opens circa > 500 files per hour (measured using lsof) without any apparent limit until it > crashes due to too many open files. > > Increasing the computer open file limit is not a solution since NiFi will > still crash, it'll only take longer to do so. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (NIFI-7114) NiFi not closing file handles
[ https://issues.apache.org/jira/browse/NIFI-7114?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vinicius Zolin updated NIFI-7114: - Attachment: openFiles.xlsx > NiFi not closing file handles > - > > Key: NIFI-7114 > URL: https://issues.apache.org/jira/browse/NIFI-7114 > Project: Apache NiFi > Issue Type: Bug > Components: Configuration >Affects Versions: 1.10.0, 1.11.0 > Environment: Amazon EC2 running either Amazon Linux 2 or Ubuntu 18.04. > NiFi has been installed with no change to any configuration file. >Reporter: Vinicius Zolin >Priority: Major > Fix For: 1.11.2 > > Attachments: destination.xml, lsof.log, lsof.zip, lsofAfter.log, > lsofBefore.log, openFiles.xlsx, source.xml > > > Since at least version 1.10 NiFi stopped closing file handles. It opens circa > 500 files per hour (measured using lsof) without any apparent limit until it > crashes due to too many open files. > > Increasing the computer open file limit is not a solution since NiFi will > still crash, it'll only take longer to do so. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NIFI-4890) OIDC Token Refresh is not done correctly
[ https://issues.apache.org/jira/browse/NIFI-4890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034564#comment-17034564 ] Matt Gilman commented on NIFI-4890: --- [~deboys] Which token are you referring to that NiFi is not honoring the expiration of? As part of the authentication, NiFi validates the ID token and extracts the expiration from its claim set. If the user identity is not part of the claim set it will use the access token to query the UserInfo endpoint. However, following that it does not use the access token from the identity provider at all. NiFi does not need to invoke endpoints on the identity provider following this authentication. Reading through the spec it states that the Refresh request may not include a new ID token [1]. Can you point to an example or something from the specification of how folks expect this to work? Thanks! [1] https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokens > OIDC Token Refresh is not done correctly > > > Key: NIFI-4890 > URL: https://issues.apache.org/jira/browse/NIFI-4890 > Project: Apache NiFi > Issue Type: Bug > Components: Core UI >Affects Versions: 1.5.0 > Environment: Environment: > Browser: Chrome / Firefox > Configuration of NiFi: > - SSL certificate for the server (no client auth) > - OIDC configuration including end_session_endpoint (see the link > https://auth.s.orchestracities.com/auth/realms/default/.well-known/openid-configuration) > >Reporter: Federico Michele Facca >Priority: Major > > It looks like the NIFI UI is not refreshing the OIDC token in background, and > because of that, when the token expires, tells you that your session is > expired. and you need to refresh the page, to get a new token. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (NIFI-7114) NiFi not closing file handles
[ https://issues.apache.org/jira/browse/NIFI-7114?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vinicius Zolin updated NIFI-7114: - Attachment: lsof.zip > NiFi not closing file handles > - > > Key: NIFI-7114 > URL: https://issues.apache.org/jira/browse/NIFI-7114 > Project: Apache NiFi > Issue Type: Bug > Components: Configuration >Affects Versions: 1.10.0, 1.11.0 > Environment: Amazon EC2 running either Amazon Linux 2 or Ubuntu 18.04. > NiFi has been installed with no change to any configuration file. >Reporter: Vinicius Zolin >Priority: Major > Fix For: 1.11.2 > > Attachments: destination.xml, lsof.log, lsof.zip, lsofAfter.log, > lsofBefore.log, source.xml > > > Since at least version 1.10 NiFi stopped closing file handles. It opens circa > 500 files per hour (measured using lsof) without any apparent limit until it > crashes due to too many open files. > > Increasing the computer open file limit is not a solution since NiFi will > still crash, it'll only take longer to do so. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NIFI-7117) Load Balance is not working
[ https://issues.apache.org/jira/browse/NIFI-7117?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034558#comment-17034558 ] Mark Payne commented on NIFI-7117: -- NIFI-7059 introduced a bug that resulted in different nodes in the cluster having a different ordering of the nodes. Load Balanced Connection partitioners assume that nodes in the cluster have the same ordering (by sorting based on the Nodes' UUIDs). This Jira addresses the bug that was introduced there. > Load Balance is not working > --- > > Key: NIFI-7117 > URL: https://issues.apache.org/jira/browse/NIFI-7117 > Project: Apache NiFi > Issue Type: Bug > Components: Core Framework >Affects Versions: 1.11.1 >Reporter: Eduardo Mota Fontes >Assignee: Mark Payne >Priority: Blocker > Fix For: 1.12.0, 1.11.2 > > Time Spent: 0.5h > Remaining Estimate: 0h > > Load Balancing is not working on 1.11.1. > I've tested with a 3 node cluster, GenerateFlowFile on All Nodes and a > connection with Single Node Load Balancing. All FlowFiles remains in its node. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Resolved] (MINIFICPP-1137) rocksdb-enabled Windows builds leak on every thread exit
[ https://issues.apache.org/jira/browse/MINIFICPP-1137?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Dániel Bakai resolved MINIFICPP-1137. - Resolution: Fixed > rocksdb-enabled Windows builds leak on every thread exit > > > Key: MINIFICPP-1137 > URL: https://issues.apache.org/jira/browse/MINIFICPP-1137 > Project: Apache NiFi MiNiFi C++ > Issue Type: Bug >Affects Versions: 0.7.0 >Reporter: Dániel Bakai >Assignee: Dániel Bakai >Priority: Blocker > Fix For: 0.8.0 > > Time Spent: 0.5h > Remaining Estimate: 0h > > rocksdb::wintlscleanup::WinOnThreadExit, the TLS cleanup callback registed by > rocksdb calls rocksdb::port::pthread_getspecific which sets errno that > (re)initializes the thread's PTD struct AFTER destroy_fls deallocated it, > resulting in the leak. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #732: MINIFICPP-1013
arpadboda commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377696233
##
File path: extensions/sql/data/Utils.cpp
##
@@ -0,0 +1,61 @@
+/**
+ *
+ * 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.
+ */
+
+#include "Utils.h"
+
+#include
+#include
+#include
+#include
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+std::string toLower(const std::string& str) {
+ std::string ret;
+
+ // (int(*)(int))std::tolower - to avoid compilation error 'no matching
overloaded function found'.
+ // It is described in
https://stackoverflow.com/questions/5539249/why-cant-transforms-begin-s-end-s-begin-tolower-be-complied-successfu.
+ std::transform(str.begin(), str.end(), std::back_inserter(ret),
(int(*)(int))std::tolower);
Review comment:
Given the IO limit we surely have in case of an SQL processor I think saving
an allocation doesn't really mean a lot.
Don't get me wrong, you are correct, but it wouldn't change anything.
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #732: MINIFICPP-1013
arpadboda commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377692105
##
File path: extensions/sql/processors/SQLProcessor.h
##
@@ -0,0 +1,105 @@
+/**
+ * @file SQLProcessor.h
+ * SQLProcessor class declaration
+ *
+ * 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.
+ */
+
+#pragma once
+
+#include "core/Core.h"
+#include "FlowFileRecord.h"
+#include "concurrentqueue.h"
+#include "core/Processor.h"
+#include "core/ProcessSession.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace processors {
+
+template
+class SQLProcessor: public core::Processor {
+ protected:
+ SQLProcessor(const std::string& name, utils::Identifier uuid)
+: core::Processor(name, uuid),
logger_(logging::LoggerFactory::getLogger()) {
+ }
+
+ void onSchedule(const std::shared_ptr& context, const
std::shared_ptr& sessionFactory) override {
+std::string controllerService;
+context->getProperty(dbControllerService().getName(), controllerService);
+
+dbService_ =
std::dynamic_pointer_cast(context->getControllerService(controllerService));
+if (!dbService_)
+ throw minifi::Exception(PROCESSOR_EXCEPTION, "'DB Controller Service'
must be defined");
+
+static_cast(this)->processOnSchedule(context, sessionFactory);
+ }
+
+ void onTrigger(const std::shared_ptr& context, const
std::shared_ptr& session) override {
+std::unique_lock lock(onTriggerMutex_, std::try_to_lock);
+if (!lock.owns_lock()) {
+ logger_->log_warn("'onTrigger' is called before previous 'onTrigger'
call is finished.");
+ context->yield();
+ return;
+}
+
+try {
+ if (!connection_) {
+connection_ = dbService_->getConnection();
+ }
+ static_cast(this)->processOnTrigger(context, session);
Review comment:
I'm also not in favour of CRTP in this case, but it doesn't hurt.
I think rewriting this part of the code would have a very little gain in
readability and a lot of time spent on it, so let's leave it as is for now.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[jira] [Created] (NIFI-7133) Clarification of EnforceOrder description
Dennis Jaheruddin created NIFI-7133: --- Summary: Clarification of EnforceOrder description Key: NIFI-7133 URL: https://issues.apache.org/jira/browse/NIFI-7133 Project: Apache NiFi Issue Type: Improvement Components: Documentation Website Reporter: Dennis Jaheruddin The EnforceOrder processor only enforces order within a single node. Though the description has some disclaimers, I would request this to be mentioned explicitly. For instance: Enforces expected ordering of FlowFiles that belong to the same data group within a single node. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NIFI-4890) OIDC Token Refresh is not done correctly
[ https://issues.apache.org/jira/browse/NIFI-4890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034495#comment-17034495 ] N commented on NIFI-4890: - is there any workaround for not honoring the tokens expirations date? > OIDC Token Refresh is not done correctly > > > Key: NIFI-4890 > URL: https://issues.apache.org/jira/browse/NIFI-4890 > Project: Apache NiFi > Issue Type: Bug > Components: Core UI >Affects Versions: 1.5.0 > Environment: Environment: > Browser: Chrome / Firefox > Configuration of NiFi: > - SSL certificate for the server (no client auth) > - OIDC configuration including end_session_endpoint (see the link > https://auth.s.orchestracities.com/auth/realms/default/.well-known/openid-configuration) > >Reporter: Federico Michele Facca >Priority: Major > > It looks like the NIFI UI is not refreshing the OIDC token in background, and > because of that, when the token expires, tells you that your session is > expired. and you need to refresh the page, to get a new token. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #732: MINIFICPP-1013
arpadboda commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377666920
##
File path: extensions/sql/services/DatabaseService.h
##
@@ -0,0 +1,116 @@
+/**
+ *
+ * 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 LIBMINIFI_INCLUDE_CONTROLLERS_DATABASESERVICE_H_
+#define LIBMINIFI_INCLUDE_CONTROLLERS_DATABASESERVICE_H_
+
+#include "core/logging/LoggerConfiguration.h"
+#include "core/controller/ControllerService.h"
+#include "data/DatabaseConnectors.h"
+#include
+#include
+
+#include
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace sql {
+namespace controllers {
+
+/**
+ * Purpose and Justification: Controller services function as a layerable way
to provide
+ * services to internal services. While a controller service is generally
configured from the flow,
+ * we want to follow the open closed principle and provide Database services
+ */
+class DatabaseService : public core::controller::ControllerService {
+ public:
+
+ /**
+ * Constructors for the controller service.
+ */
+ explicit DatabaseService(const std::string , const std::string )
+ : ControllerService(name, id),
+initialized_(false),
+logger_(logging::LoggerFactory::getLogger()) {
+initialize();
+ }
+
+ explicit DatabaseService(const std::string , utils::Identifier uuid =
utils::Identifier())
+ : ControllerService(name, uuid),
+initialized_(false),
+logger_(logging::LoggerFactory::getLogger()) {
+initialize();
+ }
+
+ explicit DatabaseService(const std::string , const
std::shared_ptr )
+ : ControllerService(name),
+initialized_(false),
+logger_(logging::LoggerFactory::getLogger()) {
+setConfiguration(configuration);
+initialize();
+ }
+
+ /**
+ * Parameters needed.
+ */
+ static core::Property ConnectionString;
+
+ virtual void initialize() override;
+
+ void yield() override {
+
+ }
+
+ bool isRunning() override {
+return getState() == core::controller::ControllerServiceState::ENABLED;
+ }
+
+ bool isWorkAvailable() override {
+return false;
+ }
+
+ virtual void onEnable() override;
+
+ virtual std::unique_ptr getConnection() const = 0;
+
+ protected:
+
+ void initializeProperties();
+
+ // initialization mutex.
+ std::recursive_mutex initialization_mutex_;
Review comment:
I also think that recursive mutex is a bad smell, but the usage looks
correct here, so let's just create a follow-up for this.
Created https://issues.apache.org/jira/browse/MINIFICPP-1156
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
With regards,
Apache Git Services
[jira] [Created] (MINIFICPP-1156) Avoid recursive mutex in Soci SQL impl
Arpad Boda created MINIFICPP-1156: - Summary: Avoid recursive mutex in Soci SQL impl Key: MINIFICPP-1156 URL: https://issues.apache.org/jira/browse/MINIFICPP-1156 Project: Apache NiFi MiNiFi C++ Issue Type: Improvement Reporter: Arpad Boda Fix For: 0.8.0 Recursive mutex is usually a bad smell and should be avoided. A possible workaround is unique locks passed as arguments. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 MINIFICPP-1154 unify win/posix sockets + fix bugs
szaszm commented on a change in pull request #713: MINIFICPP-1119
MINIFICPP-1154 unify win/posix sockets + fix bugs
URL: https://github.com/apache/nifi-minifi-cpp/pull/713#discussion_r377663356
##
File path: libminifi/include/io/ClientSocket.h
##
@@ -93,7 +109,7 @@ class Socket : public BaseStream {
*/
virtual int16_t initialize();
- virtual void setInterface(io::NetworkInterface &) {
+ virtual void setInterface(io::NetworkInterface interface) noexcept {
Review comment:
Removed `noexcept`, marking this discussion as resolved. For my arguments on
`noexcept`, refer to the thread about
`NetworkInterface::operator=(NetworkInterface &)`.
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] bakaid commented on a change in pull request #732: MINIFICPP-1013
bakaid commented on a change in pull request #732: MINIFICPP-1013 URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377663445 ## File path: win_build_vs.bat ## @@ -22,10 +22,12 @@ if [%1]==[] goto usage set builddir=%1 set skiptests=OFF set cmake_build_type=Release +set build_type=Release Review comment: Yep, this looks like a bad merge and breaks the `/D` option. 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 With regards, Apache Git Services
[GitHub] [nifi] mattyb149 commented on a change in pull request #4043: NIFI-7109 Unit tests should be able to determine if item validator wa…
mattyb149 commented on a change in pull request #4043: NIFI-7109 Unit tests
should be able to determine if item validator wa…
URL: https://github.com/apache/nifi/pull/4043#discussion_r377658720
##
File path:
nifi-commons/nifi-utils/src/test/java/org/apache/nifi/util/validator/TestMockValidator.java
##
@@ -0,0 +1,92 @@
+/*
+ * 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.util.validator;
+
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
+import org.apache.nifi.components.Validator;
+
+/**
+ * TestMockValidator wraps a {@class Validator} and provides statistics on
it's interactions.
+ * Because many of the {@class Validator} instances returned from {@class
StandardValidator }
+ * are not mockable with with mockito, this is required to know, when running
a test, if a
+ * {@class Validator} was in fact called, for example.
+ */
+public class TestMockValidator implements Validator {
Review comment:
I'm OK with it in this instance, since one is specifically for the provided
StandardValidators and the other is for custom validators. Pretty sure we've
done things like this before for the same reason (search for `MockDBCPService`
in the codebase )
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
With regards,
Apache Git Services
[GitHub] [nifi] ottobackwards commented on a change in pull request #4043: NIFI-7109 Unit tests should be able to determine if item validator wa…
ottobackwards commented on a change in pull request #4043: NIFI-7109 Unit tests
should be able to determine if item validator wa…
URL: https://github.com/apache/nifi/pull/4043#discussion_r377658345
##
File path:
nifi-commons/nifi-utils/src/test/java/org/apache/nifi/util/validator/TestMockValidator.java
##
@@ -0,0 +1,92 @@
+/*
+ * 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.util.validator;
+
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
+import org.apache.nifi.components.Validator;
+
+/**
+ * TestMockValidator wraps a {@class Validator} and provides statistics on
it's interactions.
+ * Because many of the {@class Validator} instances returned from {@class
StandardValidator }
+ * are not mockable with with mockito, this is required to know, when running
a test, if a
+ * {@class Validator} was in fact called, for example.
+ */
+public class TestMockValidator implements Validator {
Review comment:
Also, breaking the current naming scheme
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
With regards,
Apache Git Services
[GitHub] [nifi] ottobackwards commented on a change in pull request #4043: NIFI-7109 Unit tests should be able to determine if item validator wa…
ottobackwards commented on a change in pull request #4043: NIFI-7109 Unit tests
should be able to determine if item validator wa…
URL: https://github.com/apache/nifi/pull/4043#discussion_r377656585
##
File path:
nifi-commons/nifi-utils/src/test/java/org/apache/nifi/util/validator/TestMockValidator.java
##
@@ -0,0 +1,92 @@
+/*
+ * 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.util.validator;
+
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
+import org.apache.nifi.components.Validator;
+
+/**
+ * TestMockValidator wraps a {@class Validator} and provides statistics on
it's interactions.
+ * Because many of the {@class Validator} instances returned from {@class
StandardValidator }
+ * are not mockable with with mockito, this is required to know, when running
a test, if a
+ * {@class Validator} was in fact called, for example.
+ */
+public class TestMockValidator implements Validator {
Review comment:
As long as everyone is OK with the DRYness ;)
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
With regards,
Apache Git Services
[GitHub] [nifi] mattyb149 commented on a change in pull request #4043: NIFI-7109 Unit tests should be able to determine if item validator wa…
mattyb149 commented on a change in pull request #4043: NIFI-7109 Unit tests
should be able to determine if item validator wa…
URL: https://github.com/apache/nifi/pull/4043#discussion_r377655229
##
File path:
nifi-commons/nifi-utils/src/test/java/org/apache/nifi/util/validator/TestMockValidator.java
##
@@ -0,0 +1,92 @@
+/*
+ * 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.util.validator;
+
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
+import org.apache.nifi.components.Validator;
+
+/**
+ * TestMockValidator wraps a {@class Validator} and provides statistics on
it's interactions.
+ * Because many of the {@class Validator} instances returned from {@class
StandardValidator }
+ * are not mockable with with mockito, this is required to know, when running
a test, if a
+ * {@class Validator} was in fact called, for example.
+ */
+public class TestMockValidator implements Validator {
Review comment:
Makes sense, since the test is in nifi-utils (along with the
unit-under-test). Still it sounds useful in general, what about a
StandardValidators-specific one in nifi-utils (the way you have it now),
perhaps having `StandardValidator` in the name (I was thinking about something
besides `Test` that indicates you're instrumenting a real one, perhaps
`InstrumentedStandardValidator` for the one in nifi-utils, and then have
another one in nifi-mock called `InstrumentedValidator`?
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013 URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377599401 ## File path: win_build_vs.bat ## @@ -22,10 +22,12 @@ if [%1]==[] goto usage set builddir=%1 set skiptests=OFF set cmake_build_type=Release +set build_type=Release Review comment: Now that we have `built_type`, what do we use `cmake_build_type` for? Was this introduced with the rebase? Because I remember @bakaid removing something like that not long ago. 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 With regards, Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377647387
##
File path: extensions/sql/processors/QueryDatabaseTable.cpp
##
@@ -0,0 +1,475 @@
+/**
+ * @file QueryDatabaseTable.cpp
+ * PutSQL class declaration
+ *
+ * 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.
+ */
+
+#include "QueryDatabaseTable.h"
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include "io/DataStream.h"
+#include "core/ProcessContext.h"
+#include "core/ProcessSession.h"
+#include "Exception.h"
+#include "utils/OsUtils.h"
+#include "data/DatabaseConnectors.h"
+#include "data/JSONSQLWriter.h"
+#include "data/SQLRowsetProcessor.h"
+#include "data/WriteCallback.h"
+#include "data/MaxCollector.h"
+#include "data/Utils.h"
+#include "utils/file/FileUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace processors {
+
+const std::string QueryDatabaseTable::ProcessorName("QueryDatabaseTable");
+
+const core::Property QueryDatabaseTable::s_tableName(
+ core::PropertyBuilder::createProperty("Table
Name")->isRequired(true)->withDescription("The name of the database table to be
queried.")->supportsExpressionLanguage(true)->build());
+
+const core::Property QueryDatabaseTable::s_columnNames(
+ core::PropertyBuilder::createProperty("Columns to
Return")->isRequired(false)->withDescription(
+"A comma-separated list of column names to be used in the query. If your
database requires special treatment of the names (quoting, e.g.), each name
should include such treatment. "
+"If no column names are supplied, all columns in the specified table will
be returned. "
+"NOTE: It is important to use consistent column names for a given table
for incremental fetch to work
properly.")->supportsExpressionLanguage(true)->build());
+
+const core::Property QueryDatabaseTable::s_maxValueColumnNames(
+ core::PropertyBuilder::createProperty("Maximum-value
Columns")->isRequired(false)->withDescription(
+"A comma-separated list of column names. The processor will keep track of
the maximum value for each column that has been returned since the processor
started running. "
+"Using multiple columns implies an order to the column list, and each
column's values are expected to increase more slowly than the previous columns'
values. "
+"Thus, using multiple columns implies a hierarchical structure of columns,
which is usually used for partitioning tables. "
+"This processor can be used to retrieve only those rows that have been
added/updated since the last retrieval. "
+"Note that some ODBC types such as bit/boolean are not conducive to
maintaining maximum value, so columns of these types should not be listed in
this property, and will result in error(s) during processing. "
+"If no columns are provided, all rows from the table will be considered,
which could have a performance impact. "
+"NOTE: It is important to use consistent max-value column names for a
given table for incremental fetch to work properly. "
+"NOTE: Because of a limitation of database access library 'soci', which
doesn't support milliseconds in it's 'dt_date', "
+"there is a possibility that flowfiles might have duplicated records, if a
max-value column with 'dt_date' type has value with milliseconds.")->
+supportsExpressionLanguage(true)->build());
+
+const core::Property QueryDatabaseTable::s_whereClause(
+
core::PropertyBuilder::createProperty("db-fetch-where-clause")->isRequired(false)->withDescription(
+"A custom clause to be added in the WHERE condition when building SQL
queries.")->supportsExpressionLanguage(true)->build());
+
+const core::Property QueryDatabaseTable::s_sqlQuery(
+
core::PropertyBuilder::createProperty("db-fetch-sql-query")->isRequired(false)->withDescription(
+"A custom SQL query used to retrieve data. Instead of building a SQL query
from other properties, this query will be wrapped as a sub-query. "
+"Query must have no ORDER BY
statement.")->supportsExpressionLanguage(true)->build());
+
+const
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013 URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377575687 ## File path: extensions/sql/data/DatabaseConnectors.h ## @@ -0,0 +1,106 @@ +/** + * + * 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 EXTENSIONS_SQL_SERVICES_DATABASECONNECTORS_H_ +#define EXTENSIONS_SQL_SERVICES_DATABASECONNECTORS_H_ + +#include +#include +#include +#include Review comment: I don't see anything from these 3 headers being used in this header. We are missing ``, though. 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 With regards, Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377573687
##
File path: extensions/sql/SQLLoader.h
##
@@ -0,0 +1,81 @@
+/**
+ *
+ * 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 EXTENSION_SQLLOADER_H
+#define EXTENSION_SQLLOADER_H
+
+#include "core/ClassLoader.h"
+#include "processors/ExecuteSQL.h"
+#include "processors/PutSQL.h"
+#include "processors/QueryDatabaseTable.h"
+#include "services/ODBCConnector.h"
+
+class SQLFactory : public core::ObjectFactory {
+ public:
+ SQLFactory() {
+
+ }
Review comment:
Too much vertical space reduces code readability. I suggest `SQLFactory() =
default;`.
There are more occurrences of more vertical space used than I prefer, but
since this is a question of taste, I'll leave it up to you. In this case using
3 lines for something that's logically only 1 went over my threshold of
commenting.
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377587225
##
File path: extensions/sql/data/Utils.cpp
##
@@ -0,0 +1,61 @@
+/**
+ *
+ * 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.
+ */
+
+#include "Utils.h"
+
+#include
+#include
+#include
+#include
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+std::string toLower(const std::string& str) {
+ std::string ret;
+
+ // (int(*)(int))std::tolower - to avoid compilation error 'no matching
overloaded function found'.
+ // It is described in
https://stackoverflow.com/questions/5539249/why-cant-transforms-begin-s-end-s-begin-tolower-be-complied-successfu.
+ std::transform(str.begin(), str.end(), std::back_inserter(ret),
(int(*)(int))std::tolower);
Review comment:
Taking `str` by value and using `transform` in-place would avoid additional
allocations and make it possible for users of the function to move in their
long strings if they only need the lowercase version.
I think a lambda that selects the appropriate overload looks nicer than a
function pointer cast, but that's subjective.
I like the fact that you used STL to accomplish this. Generic algorithms are
beautiful and underused IMO so I appreciate every problem solved in terms of
existing generic algorithms.
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377646615
##
File path: extensions/sql/processors/QueryDatabaseTable.cpp
##
@@ -0,0 +1,475 @@
+/**
+ * @file QueryDatabaseTable.cpp
+ * PutSQL class declaration
+ *
+ * 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.
+ */
+
+#include "QueryDatabaseTable.h"
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include "io/DataStream.h"
+#include "core/ProcessContext.h"
+#include "core/ProcessSession.h"
+#include "Exception.h"
+#include "utils/OsUtils.h"
+#include "data/DatabaseConnectors.h"
+#include "data/JSONSQLWriter.h"
+#include "data/SQLRowsetProcessor.h"
+#include "data/WriteCallback.h"
+#include "data/MaxCollector.h"
+#include "data/Utils.h"
+#include "utils/file/FileUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace processors {
+
+const std::string QueryDatabaseTable::ProcessorName("QueryDatabaseTable");
+
+const core::Property QueryDatabaseTable::s_tableName(
+ core::PropertyBuilder::createProperty("Table
Name")->isRequired(true)->withDescription("The name of the database table to be
queried.")->supportsExpressionLanguage(true)->build());
+
+const core::Property QueryDatabaseTable::s_columnNames(
+ core::PropertyBuilder::createProperty("Columns to
Return")->isRequired(false)->withDescription(
+"A comma-separated list of column names to be used in the query. If your
database requires special treatment of the names (quoting, e.g.), each name
should include such treatment. "
+"If no column names are supplied, all columns in the specified table will
be returned. "
+"NOTE: It is important to use consistent column names for a given table
for incremental fetch to work
properly.")->supportsExpressionLanguage(true)->build());
+
+const core::Property QueryDatabaseTable::s_maxValueColumnNames(
+ core::PropertyBuilder::createProperty("Maximum-value
Columns")->isRequired(false)->withDescription(
+"A comma-separated list of column names. The processor will keep track of
the maximum value for each column that has been returned since the processor
started running. "
+"Using multiple columns implies an order to the column list, and each
column's values are expected to increase more slowly than the previous columns'
values. "
+"Thus, using multiple columns implies a hierarchical structure of columns,
which is usually used for partitioning tables. "
+"This processor can be used to retrieve only those rows that have been
added/updated since the last retrieval. "
+"Note that some ODBC types such as bit/boolean are not conducive to
maintaining maximum value, so columns of these types should not be listed in
this property, and will result in error(s) during processing. "
+"If no columns are provided, all rows from the table will be considered,
which could have a performance impact. "
+"NOTE: It is important to use consistent max-value column names for a
given table for incremental fetch to work properly. "
+"NOTE: Because of a limitation of database access library 'soci', which
doesn't support milliseconds in it's 'dt_date', "
+"there is a possibility that flowfiles might have duplicated records, if a
max-value column with 'dt_date' type has value with milliseconds.")->
+supportsExpressionLanguage(true)->build());
+
+const core::Property QueryDatabaseTable::s_whereClause(
+
core::PropertyBuilder::createProperty("db-fetch-where-clause")->isRequired(false)->withDescription(
+"A custom clause to be added in the WHERE condition when building SQL
queries.")->supportsExpressionLanguage(true)->build());
+
+const core::Property QueryDatabaseTable::s_sqlQuery(
+
core::PropertyBuilder::createProperty("db-fetch-sql-query")->isRequired(false)->withDescription(
+"A custom SQL query used to retrieve data. Instead of building a SQL query
from other properties, this query will be wrapped as a sub-query. "
+"Query must have no ORDER BY
statement.")->supportsExpressionLanguage(true)->build());
+
+const
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377577407
##
File path: extensions/sql/data/JSONSQLWriter.h
##
@@ -0,0 +1,65 @@
+/**
+ *
+ * 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.
+ */
+
+#pragma once
+
+#include "rapidjson/document.h"
+
+#include "SQLWriter.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace sql {
+
+class JSONSQLWriter: public SQLWriter {
+ public:
+ JSONSQLWriter(bool pretty);
Review comment:
Single parameter constructors should either be marked `explicit` or have a
comment explaining why implicit conversion from the parameter type is intended.
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377585714
##
File path: extensions/sql/data/SQLRowsetProcessor.h
##
@@ -0,0 +1,62 @@
+/**
+ *
+ * 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.
+ */
+
+#pragma once
+
+#include
+
+#include
+
+#include "SQLRowSubscriber.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace sql {
+
+class SQLRowsetProcessor
+{
+ public:
+ SQLRowsetProcessor(const soci::rowset& rowset, const
std::vector& rowSubscribers);
+
+ size_t process(size_t max = 0);
Review comment:
The meaning of `max == 0` needs to be documented.
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377630872
##
File path: extensions/sql/processors/SQLProcessor.h
##
@@ -0,0 +1,105 @@
+/**
+ * @file SQLProcessor.h
+ * SQLProcessor class declaration
+ *
+ * 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.
+ */
+
+#pragma once
+
+#include "core/Core.h"
+#include "FlowFileRecord.h"
+#include "concurrentqueue.h"
+#include "core/Processor.h"
+#include "core/ProcessSession.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace processors {
+
+template
+class SQLProcessor: public core::Processor {
+ protected:
+ SQLProcessor(const std::string& name, utils::Identifier uuid)
+: core::Processor(name, uuid),
logger_(logging::LoggerFactory::getLogger()) {
+ }
+
+ void onSchedule(const std::shared_ptr& context, const
std::shared_ptr& sessionFactory) override {
+std::string controllerService;
+context->getProperty(dbControllerService().getName(), controllerService);
+
+dbService_ =
std::dynamic_pointer_cast(context->getControllerService(controllerService));
+if (!dbService_)
+ throw minifi::Exception(PROCESSOR_EXCEPTION, "'DB Controller Service'
must be defined");
+
+static_cast(this)->processOnSchedule(context, sessionFactory);
+ }
+
+ void onTrigger(const std::shared_ptr& context, const
std::shared_ptr& session) override {
+std::unique_lock lock(onTriggerMutex_, std::try_to_lock);
+if (!lock.owns_lock()) {
+ logger_->log_warn("'onTrigger' is called before previous 'onTrigger'
call is finished.");
+ context->yield();
+ return;
+}
+
+try {
+ if (!connection_) {
+connection_ = dbService_->getConnection();
+ }
+ static_cast(this)->processOnTrigger(context, session);
Review comment:
I don't think we need CRTP to avoid the overhead of a virtual function call,
given that we abuse shared_ptr all over our codebase and nobody bats an eye.
I suggest making `processOnTrigger` and `processOnSchedule` pure virtual in
`SQLProcessor`, `final` in the implementations and not use CRTP. This will also
prevent accidental instantiation of `SQLProcessor`, and it seems to me that it
was not meant to be used directly, so that's positive in this case.
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377593640
##
File path: extensions/sql/processors/QueryDatabaseTable.cpp
##
@@ -0,0 +1,475 @@
+/**
+ * @file QueryDatabaseTable.cpp
+ * PutSQL class declaration
+ *
+ * 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.
+ */
+
+#include "QueryDatabaseTable.h"
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include "io/DataStream.h"
+#include "core/ProcessContext.h"
+#include "core/ProcessSession.h"
+#include "Exception.h"
+#include "utils/OsUtils.h"
+#include "data/DatabaseConnectors.h"
+#include "data/JSONSQLWriter.h"
+#include "data/SQLRowsetProcessor.h"
+#include "data/WriteCallback.h"
+#include "data/MaxCollector.h"
+#include "data/Utils.h"
+#include "utils/file/FileUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace processors {
+
+const std::string QueryDatabaseTable::ProcessorName("QueryDatabaseTable");
+
+const core::Property QueryDatabaseTable::s_tableName(
+ core::PropertyBuilder::createProperty("Table
Name")->isRequired(true)->withDescription("The name of the database table to be
queried.")->supportsExpressionLanguage(true)->build());
+
+const core::Property QueryDatabaseTable::s_columnNames(
+ core::PropertyBuilder::createProperty("Columns to
Return")->isRequired(false)->withDescription(
+"A comma-separated list of column names to be used in the query. If your
database requires special treatment of the names (quoting, e.g.), each name
should include such treatment. "
+"If no column names are supplied, all columns in the specified table will
be returned. "
+"NOTE: It is important to use consistent column names for a given table
for incremental fetch to work
properly.")->supportsExpressionLanguage(true)->build());
+
+const core::Property QueryDatabaseTable::s_maxValueColumnNames(
+ core::PropertyBuilder::createProperty("Maximum-value
Columns")->isRequired(false)->withDescription(
+"A comma-separated list of column names. The processor will keep track of
the maximum value for each column that has been returned since the processor
started running. "
+"Using multiple columns implies an order to the column list, and each
column's values are expected to increase more slowly than the previous columns'
values. "
+"Thus, using multiple columns implies a hierarchical structure of columns,
which is usually used for partitioning tables. "
+"This processor can be used to retrieve only those rows that have been
added/updated since the last retrieval. "
+"Note that some ODBC types such as bit/boolean are not conducive to
maintaining maximum value, so columns of these types should not be listed in
this property, and will result in error(s) during processing. "
+"If no columns are provided, all rows from the table will be considered,
which could have a performance impact. "
+"NOTE: It is important to use consistent max-value column names for a
given table for incremental fetch to work properly. "
+"NOTE: Because of a limitation of database access library 'soci', which
doesn't support milliseconds in it's 'dt_date', "
+"there is a possibility that flowfiles might have duplicated records, if a
max-value column with 'dt_date' type has value with milliseconds.")->
+supportsExpressionLanguage(true)->build());
+
+const core::Property QueryDatabaseTable::s_whereClause(
+
core::PropertyBuilder::createProperty("db-fetch-where-clause")->isRequired(false)->withDescription(
+"A custom clause to be added in the WHERE condition when building SQL
queries.")->supportsExpressionLanguage(true)->build());
+
+const core::Property QueryDatabaseTable::s_sqlQuery(
+
core::PropertyBuilder::createProperty("db-fetch-sql-query")->isRequired(false)->withDescription(
+"A custom SQL query used to retrieve data. Instead of building a SQL query
from other properties, this query will be wrapped as a sub-query. "
+"Query must have no ORDER BY
statement.")->supportsExpressionLanguage(true)->build());
+
+const
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377606863
##
File path: extensions/sql/data/DatabaseConnectors.h
##
@@ -0,0 +1,106 @@
+/**
+ *
+ * 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 EXTENSIONS_SQL_SERVICES_DATABASECONNECTORS_H_
+#define EXTENSIONS_SQL_SERVICES_DATABASECONNECTORS_H_
+
+#include
+#include
+#include
+#include
+
+#include
+
+#include "Utils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace sql {
+
+/**
+ * We do not intend to create an abstract facade here. We know that SOCI is
the underlying
+ * SQL library. We only wish to abstract ODBC specific information
+ */
+
+class Statement {
+ public:
+
+ explicit Statement(const std::unique_ptr& session, const
std::string )
Review comment:
If we need a reference to `soci::session`, then we should take a
`soci::session&`, not a `const std::unique_ptr&`. We expect
`session` to be non-null and there's no reason to enforce unique ownership on
the caller, an observer reference/pointer should be enough.
I didn't create multiple review comments for all the occurrences of this
same issue. Same with shared_ptr if shared ownership is not desired.
Some I noticed:
- `Session::Session`
- `PutSQL::processOnSchedule`
- `PutSQL::processOnTrigger`
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377573255
##
File path: extensions/sql/SQLLoader.h
##
@@ -0,0 +1,81 @@
+/**
+ *
+ * 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 EXTENSION_SQLLOADER_H
+#define EXTENSION_SQLLOADER_H
+
+#include "core/ClassLoader.h"
+#include "processors/ExecuteSQL.h"
+#include "processors/PutSQL.h"
+#include "processors/QueryDatabaseTable.h"
+#include "services/ODBCConnector.h"
+
+class SQLFactory : public core::ObjectFactory {
+ public:
+ SQLFactory() {
+
+ }
+
+ /**
+ * Gets the name of the object.
+ * @return class name of processor
+ */
+ virtual std::string getName() override {
+return "SQLFactory";
+ }
+
+ virtual std::string getClassName() override{
+return "SQLFactory";
+ }
+ /**
+ * Gets the class name for the object
+ * @return class name for the processor.
+ */
+ virtual std::vector getClassNames() override{
Review comment:
I'm in favor of following "Virtual functions should specify exactly one of
`virtual`, `override`, or `final`" from the C++ Core Guidelines.
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rh-override
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377584537
##
File path: extensions/sql/data/MaxCollector.h
##
@@ -0,0 +1,172 @@
+/**
+ *
+ * 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.
+ */
+
+#pragma once
+
+#include
+#include
+#include
+
+#include "SQLRowSubscriber.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace sql {
+
+class MaxCollector: public SQLRowSubscriber {
+ void beginProcessRow() override {}
+
+ void endProcessRow() override {
+if (columnsVerified_) {
+ return;
+}
+
+if (countColumns_ != mapState_.size())
+ throw minifi::Exception(PROCESSOR_EXCEPTION, "MaxCollector: Column(s) '"
+ maxValueColumnNames_ + "' are not found in the columns of '" + selectQuery_ +
"' result.");
+
+columnsVerified_ = true;
+ }
+
+ void processColumnName(const std::string& name) override {
+if (columnsVerified_) {
+ return;
+}
+
+if (mapState_.count(name)) {
+ countColumns_++;
+}
+ }
+
+ void processColumn(const std::string& name, const std::string& value)
override {
+updateMaxValue(name, '\'' + value + '\'');
+ }
+
+ void processColumn(const std::string& name, double value) override {
+updateMaxValue(name, value);
+ }
+
+ void processColumn(const std::string& name, int value) override {
+updateMaxValue(name, value);
+ }
+
+ void processColumn(const std::string& name, long long value) override {
+updateMaxValue(name, value);
+ }
+
+ void processColumn(const std::string& name, unsigned long long value)
override {
+updateMaxValue(name, value);
+ }
+
+ void processColumn(const std::string& name, const char* value) override {}
+
+ template
+ struct MaxValue {
+void updateMaxValue(const std::string& name, const T& value) {
+ const auto it = mapColumnNameValue_.find(name);
+ if (it == mapColumnNameValue_.end()) {
+mapColumnNameValue_.insert({ name, value });
+ } else {
+if (value > it->second) {
+ it->second = value;
+}
+ }
+}
+
+std::unordered_map mapColumnNameValue_;
+ };
+
+ template
+ struct TupleIndexByType {
+constexpr static int index() {
+ using tupleElType = typename std::decay(Tuple()))>::type;
+
+ return TupleIndexByType>::value>::index();
+}
+ };
+
+ template
+ struct TupleIndexByType {
+constexpr static int index() {
+ return Index;
+}
+ };
+
+ template
+ struct UpdateMapState {
+UpdateMapState(const Tuple& tpl, std::unordered_map& mapState) {
+ for (auto& el : mapState) {
+const auto& maxVal = std::get(tpl);
+
+const auto it = maxVal.mapColumnNameValue_.find(el.first);
+if (it != maxVal.mapColumnNameValue_.end()) {
+ std::stringstream ss;
+ ss << it->second;
+ el.second = ss.str();
+}
+ }
+
+ UpdateMapState(tpl, mapState);
+}
+ };
+
+ template
+ struct UpdateMapState {
+UpdateMapState(const Tuple&, std::unordered_map&) {}
+ };
+
+ template
+ struct MaxValues : public std::tuple...> {
+constexpr static size_t size = sizeof...(Ts);
+ };
+
+ public:
+ MaxCollector(const std::string& selectQuery, const std::string&
maxValueColumnNames, std::unordered_map& mapState)
+:selectQuery_(selectQuery), maxValueColumnNames_(maxValueColumnNames),
mapState_(mapState) {
+ }
+
+ template
+ void updateMaxValue(const std::string& columnName, const T& value) {
+if (mapState_.count(columnName)) {
+ constexpr auto index = TupleIndexByType::index();
+ std::get(maxValues_).updateMaxValue(columnName, value);
+}
+ }
+
+ bool updateMapState() {
+auto mapState = mapState_;
+UpdateMapState(maxValues_, mapState_);
+
+return mapState != mapState_;
Review comment:
The fact that we copy the whole map here to see if there were changes while
make the code way more verbose to avoid additional lookups/writes in `MaxValue`
is contradictory and feels wrong.
Can we either optimize properly for performance if it matters or reduce
verbosity if it doesn't?
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377572695
##
File path: extensions/sql/SQLLoader.h
##
@@ -0,0 +1,81 @@
+/**
+ *
+ * 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 EXTENSION_SQLLOADER_H
+#define EXTENSION_SQLLOADER_H
+
+#include "core/ClassLoader.h"
+#include "processors/ExecuteSQL.h"
+#include "processors/PutSQL.h"
+#include "processors/QueryDatabaseTable.h"
+#include "services/ODBCConnector.h"
+
+class SQLFactory : public core::ObjectFactory {
+ public:
+ SQLFactory() {
+
+ }
+
+ /**
+ * Gets the name of the object.
+ * @return class name of processor
+ */
+ virtual std::string getName() override {
+return "SQLFactory";
+ }
+
+ virtual std::string getClassName() override{
+return "SQLFactory";
+ }
+ /**
+ * Gets the class name for the object
+ * @return class name for the processor.
+ */
+ virtual std::vector getClassNames() override{
+std::vector class_names = {"ExecuteSQL", "PutSQL",
"QueryDatabaseTable", "ODBCService"};
+return class_names;
+ }
+
+ template
+ static std::unique_ptr getObjectFactory() {
+return std::unique_ptr(new core::DefautObjectFactory());
+ }
+
+ virtual std::unique_ptr assign(const std::string _name)
override {
+if (utils::StringUtils::equalsIgnoreCase(class_name, "ExecuteSQL")) {
+ return getObjectFactory();
+}
+if (utils::StringUtils::equalsIgnoreCase(class_name, "PutSQL")) {
+ return getObjectFactory();
+}
+if (utils::StringUtils::equalsIgnoreCase(class_name,
"QueryDatabaseTable")) {
+ return getObjectFactory();
+}
+if (utils::StringUtils::equalsIgnoreCase(class_name, "ODBCService")) {
+ return getObjectFactory();
+}
+
+return nullptr;
+ }
+
+ static bool added;
Review comment:
I think it's a bad idea to expose a mutable variable on the public
interface. If there's a good reason for it, please add a comment explaining
that reason!
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377596564
##
File path: extensions/sql/services/DatabaseService.h
##
@@ -0,0 +1,116 @@
+/**
+ *
+ * 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 LIBMINIFI_INCLUDE_CONTROLLERS_DATABASESERVICE_H_
+#define LIBMINIFI_INCLUDE_CONTROLLERS_DATABASESERVICE_H_
+
+#include "core/logging/LoggerConfiguration.h"
+#include "core/controller/ControllerService.h"
+#include "data/DatabaseConnectors.h"
+#include
+#include
+
+#include
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace sql {
+namespace controllers {
+
+/**
+ * Purpose and Justification: Controller services function as a layerable way
to provide
+ * services to internal services. While a controller service is generally
configured from the flow,
+ * we want to follow the open closed principle and provide Database services
+ */
+class DatabaseService : public core::controller::ControllerService {
+ public:
+
+ /**
+ * Constructors for the controller service.
+ */
+ explicit DatabaseService(const std::string , const std::string )
+ : ControllerService(name, id),
+initialized_(false),
+logger_(logging::LoggerFactory::getLogger()) {
+initialize();
+ }
+
+ explicit DatabaseService(const std::string , utils::Identifier uuid =
utils::Identifier())
+ : ControllerService(name, uuid),
+initialized_(false),
+logger_(logging::LoggerFactory::getLogger()) {
+initialize();
+ }
+
+ explicit DatabaseService(const std::string , const
std::shared_ptr )
+ : ControllerService(name),
+initialized_(false),
+logger_(logging::LoggerFactory::getLogger()) {
+setConfiguration(configuration);
+initialize();
+ }
+
+ /**
+ * Parameters needed.
+ */
+ static core::Property ConnectionString;
+
+ virtual void initialize() override;
+
+ void yield() override {
+
+ }
+
+ bool isRunning() override {
+return getState() == core::controller::ControllerServiceState::ENABLED;
+ }
+
+ bool isWorkAvailable() override {
+return false;
+ }
+
+ virtual void onEnable() override;
+
+ virtual std::unique_ptr getConnection() const = 0;
+
+ protected:
+
+ void initializeProperties();
+
+ // initialization mutex.
+ std::recursive_mutex initialization_mutex_;
Review comment:
`recursive_mutex` is a smell. Please use `mutex` in new code and design your
classes appropriately!
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377579639
##
File path: extensions/sql/data/MaxCollector.h
##
@@ -0,0 +1,172 @@
+/**
+ *
+ * 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.
+ */
+
+#pragma once
+
+#include
+#include
+#include
+
+#include "SQLRowSubscriber.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace sql {
+
+class MaxCollector: public SQLRowSubscriber {
+ void beginProcessRow() override {}
+
+ void endProcessRow() override {
+if (columnsVerified_) {
+ return;
+}
+
+if (countColumns_ != mapState_.size())
+ throw minifi::Exception(PROCESSOR_EXCEPTION, "MaxCollector: Column(s) '"
+ maxValueColumnNames_ + "' are not found in the columns of '" + selectQuery_ +
"' result.");
+
+columnsVerified_ = true;
+ }
+
+ void processColumnName(const std::string& name) override {
+if (columnsVerified_) {
+ return;
+}
+
+if (mapState_.count(name)) {
+ countColumns_++;
+}
+ }
+
+ void processColumn(const std::string& name, const std::string& value)
override {
+updateMaxValue(name, '\'' + value + '\'');
+ }
+
+ void processColumn(const std::string& name, double value) override {
+updateMaxValue(name, value);
+ }
+
+ void processColumn(const std::string& name, int value) override {
+updateMaxValue(name, value);
+ }
+
+ void processColumn(const std::string& name, long long value) override {
+updateMaxValue(name, value);
+ }
+
+ void processColumn(const std::string& name, unsigned long long value)
override {
+updateMaxValue(name, value);
+ }
+
+ void processColumn(const std::string& name, const char* value) override {}
+
+ template
+ struct MaxValue {
+void updateMaxValue(const std::string& name, const T& value) {
+ const auto it = mapColumnNameValue_.find(name);
+ if (it == mapColumnNameValue_.end()) {
+mapColumnNameValue_.insert({ name, value });
+ } else {
+if (value > it->second) {
+ it->second = value;
+}
+ }
+}
+
+std::unordered_map mapColumnNameValue_;
+ };
+
+ template
+ struct TupleIndexByType {
+constexpr static int index() {
+ using tupleElType = typename std::decay(Tuple()))>::type;
+
+ return TupleIndexByType>::value>::index();
+}
+ };
+
+ template
+ struct TupleIndexByType {
+constexpr static int index() {
+ return Index;
+}
+ };
Review comment:
`std::get(std::tuple)` supports indexing by type if the type is present in
the tuple exactly once, but only since C++14. Can we/do we want to raise the
requirements of this extension to avoid reimplementing this?
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377635439
##
File path: extensions/sql/services/ODBCConnector.h
##
@@ -0,0 +1,127 @@
+/**
+ *
+ * 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.
+ */
+
+#pragma once
+
+#include "core/logging/LoggerConfiguration.h"
+#include "core/controller/ControllerService.h"
+
+#include "DatabaseService.h"
+#include "core/Resource.h"
+#include "data/DatabaseConnectors.h"
+#include
+#include
+
+#include
+#include
+
+#include
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace sql {
+namespace controllers {
+
+class ODBCConnection : public sql::Connection {
+ public:
+ explicit ODBCConnection(const std::string& connectionString)
+: connection_string_(connectionString) {
+ session_ = std::make_unique(getSessionParameters());
+ }
+
+ virtual ~ODBCConnection() {
+ }
+
+ bool connected(std::string& exception) const override {
+try {
+ exception.clear();
+ // According to
https://stackoverflow.com/questions/3668506/efficient-sql-test-query-or-validation-query-that-will-work-across-all-or-most
by Rob Hruska,
+ // 'select 1' works for: H2, MySQL, Microsoft SQL Server, PostgreSQL,
SQLite. For Orcale 'SELECT 1 FROM DUAL' works.
+ prepareStatement("select 1")->execute();
+ return true;
+} catch (std::exception& e) {
+ exception = e.what();
+ return false;
+}
+ }
+
+ std::unique_ptr prepareStatement(const std::string& query)
const override {
+return std::make_unique(session_, query);
+ }
+
+ std::unique_ptr getSession() const override {
+return std::make_unique(session_);
+ }
+
+ private:
+ const soci::connection_parameters getSessionParameters() const {
Review comment:
Top-level `const` in function return types is redundant and misleading. A
function return value will not behave as `const` since it behaves as if it was
copied/moved to the caller, therefore the `const` qualifier of the destination
takes precedence and the qualifier on the function return type is ignored.
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377598410
##
File path: extensions/sql/services/ODBCConnector.h
##
@@ -0,0 +1,127 @@
+/**
+ *
+ * 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.
+ */
+
+#pragma once
+
+#include "core/logging/LoggerConfiguration.h"
+#include "core/controller/ControllerService.h"
+
+#include "DatabaseService.h"
+#include "core/Resource.h"
+#include "data/DatabaseConnectors.h"
+#include
+#include
+
+#include
+#include
+
+#include
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace sql {
+namespace controllers {
+
+class ODBCConnection : public sql::Connection {
+ public:
+ explicit ODBCConnection(const std::string& connectionString)
+: connection_string_(connectionString) {
+ session_ = std::make_unique(getSessionParameters());
+ }
+
+ virtual ~ODBCConnection() {
+ }
+
+ bool connected(std::string& exception) const override {
+try {
+ exception.clear();
+ // According to
https://stackoverflow.com/questions/3668506/efficient-sql-test-query-or-validation-query-that-will-work-across-all-or-most
by Rob Hruska,
+ // 'select 1' works for: H2, MySQL, Microsoft SQL Server, PostgreSQL,
SQLite. For Orcale 'SELECT 1 FROM DUAL' works.
Review comment:
typo: s/Orcale/Oracle/
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #732: MINIFICPP-1013
szaszm commented on a change in pull request #732: MINIFICPP-1013
URL: https://github.com/apache/nifi-minifi-cpp/pull/732#discussion_r377602791
##
File path: extensions/sql/SQLLoader.h
##
@@ -0,0 +1,81 @@
+/**
+ *
+ * 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 EXTENSION_SQLLOADER_H
+#define EXTENSION_SQLLOADER_H
+
+#include "core/ClassLoader.h"
+#include "processors/ExecuteSQL.h"
+#include "processors/PutSQL.h"
+#include "processors/QueryDatabaseTable.h"
+#include "services/ODBCConnector.h"
+
+class SQLFactory : public core::ObjectFactory {
+ public:
+ SQLFactory() {
+
+ }
+
+ /**
+ * Gets the name of the object.
+ * @return class name of processor
+ */
+ virtual std::string getName() override {
+return "SQLFactory";
+ }
+
+ virtual std::string getClassName() override{
+return "SQLFactory";
+ }
+ /**
+ * Gets the class name for the object
+ * @return class name for the processor.
+ */
+ virtual std::vector getClassNames() override{
+std::vector class_names = {"ExecuteSQL", "PutSQL",
"QueryDatabaseTable", "ODBCService"};
+return class_names;
+ }
+
+ template
+ static std::unique_ptr getObjectFactory() {
+return std::unique_ptr(new core::DefautObjectFactory());
Review comment:
`unique_ptr` is implicitly convertible from a `unique_ptr` to a compatible
type, so you can use `make_unique` if you want to avoid spelling the type name
one more time. This is again a question of taste, so feel free to
ignore/resolve this.
If you want to keep this extension C++11-compatible, I've made a
`make_unique` impl in GeneralUtils.h.
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
With regards,
Apache Git Services
[GitHub] [nifi] ottobackwards commented on a change in pull request #4043: NIFI-7109 Unit tests should be able to determine if item validator wa…
ottobackwards commented on a change in pull request #4043: NIFI-7109 Unit tests
should be able to determine if item validator wa…
URL: https://github.com/apache/nifi/pull/4043#discussion_r377649521
##
File path:
nifi-commons/nifi-utils/src/test/java/org/apache/nifi/util/validator/TestMockValidator.java
##
@@ -0,0 +1,92 @@
+/*
+ * 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.util.validator;
+
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
+import org.apache.nifi.components.Validator;
+
+/**
+ * TestMockValidator wraps a {@class Validator} and provides statistics on
it's interactions.
+ * Because many of the {@class Validator} instances returned from {@class
StandardValidator }
+ * are not mockable with with mockito, this is required to know, when running
a test, if a
+ * {@class Validator} was in fact called, for example.
+ */
+public class TestMockValidator implements Validator {
Review comment:
@mattyb149, I tried to move it, and add a test stop dependency to nifi-mock
to nifi-utils, I get cyclic dependency errors and cannot build
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
With regards,
Apache Git Services
[jira] [Updated] (MINIFICPP-1145) Create a common RocksdbRepository baseclass
[ https://issues.apache.org/jira/browse/MINIFICPP-1145?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Arpad Boda updated MINIFICPP-1145: -- Description: As MINIFICPP-1126 and MINIFICPP-1127 highlighted there is a lot in common in Provenance and FlowFile repositories. Rocksdb-based content repo and the planned rocksdb-based processor state storage would also result in some code duplication. printStats should be moved to the baseclass and properly handle failed getter calls. was: As MINIFICPP-1126 and MINIFICPP-1127 highlighted there is a lot in common in Provenance and FlowFile repositories. Rocksdb-based content repo and the planned rocksdb-based processor state storage would also result in some code duplication. > Create a common RocksdbRepository baseclass > --- > > Key: MINIFICPP-1145 > URL: https://issues.apache.org/jira/browse/MINIFICPP-1145 > Project: Apache NiFi MiNiFi C++ > Issue Type: Improvement >Affects Versions: 0.7.0 >Reporter: Arpad Boda >Assignee: Arpad Boda >Priority: Major > Fix For: 0.8.0 > > > As MINIFICPP-1126 and MINIFICPP-1127 highlighted there is a lot in common in > Provenance and FlowFile repositories. > Rocksdb-based content repo and the planned rocksdb-based processor state > storage would also result in some code duplication. > printStats should be moved to the baseclass and properly handle failed getter > calls. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [nifi-minifi-cpp] arpadboda commented on issue #733: MINIFICPP-1155 - Rocksdb repos print invalid stats
arpadboda commented on issue #733: MINIFICPP-1155 - Rocksdb repos print invalid stats URL: https://github.com/apache/nifi-minifi-cpp/pull/733#issuecomment-584619368 Added a note to https://issues.apache.org/jira/browse/MINIFICPP-1145 to handle this nicely in the refactor 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 With regards, Apache Git Services
