[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user mattyb149 commented on the issue: https://github.com/apache/nifi/pull/1662 Also I ran a full build with unit tests, and tried various scripts and features to exercise the processor. ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user mattyb149 commented on the issue: https://github.com/apache/nifi/pull/1662 +1 LGTM, thanks for sticking with this, it's a great addition! I made one final commit for some CheckStyle and typo stuff (and added displayName() to the properties), then rebased into one commit with you as the author. Thanks again for this feature! Merging to master ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user dlukyanov commented on the issue: https://github.com/apache/nifi/pull/1662 mattyb149, i just refactored the the `CTL` properties: - 'CTL' provides direct access to controller services without any additional logic - a new map named `SQL` provides fast to transactional DBCP service ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user dlukyanov commented on the issue: https://github.com/apache/nifi/pull/1662 mattyb149, rebased against the latest master 1.5.0-SNAPSHOT ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user dlukyanov commented on the issue: https://github.com/apache/nifi/pull/1662 @mattyb149, do you have any comments to my last commit? ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user dlukyanov commented on the issue: https://github.com/apache/nifi/pull/1662 @mattyb149, I fixed everything except: - Failure strategy this property does not limit anything in error handling. it provides default algorithms for non-handled exceptions. the user still can do try-catch-transfer in code. i beleive i've added enough description of this property. ready for discussion about this point. - rightShift i really like it, but the main reason why i did not do it - that in similar cases in groovy only `leftShift` used. -- http://docs.groovy-lang.org/latest/html/groovy-jdk/java/io/OutputStream.html -- http://docs.groovy-lang.org/latest/html/groovy-jdk/java/io/Writer.html -- http://docs.groovy-lang.org/latest/html/groovy-jdk/java/util/Collection.html --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user dlukyanov commented on the issue: https://github.com/apache/nifi/pull/1662 @mattyb149 , done. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user mattyb149 commented on the issue: https://github.com/apache/nifi/pull/1662 Sorry for the delay, I've been meaning to get back to this review. Unfortunately in the meantime the master branch has moved to version 1.4.0-SNAPSHOT, can you change your references from 1.3.0-SNAPSHOT to 1.4.0-SNAPSHOT and rebase against the latest master? Please and thank you! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user mattyb149 commented on the issue: https://github.com/apache/nifi/pull/1662 Don't worry about test failures in other parts of the project, they can be handled with other Jira cases and fixes. When I review your PR I build and test the relevant parts, and ensure a successful full build (whether all the tests pass or not). Feel free to revert those commits and rebase the whole thing into one, will make it easier to review/merge, please and thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user dlukyanov commented on the issue: https://github.com/apache/nifi/pull/1662 I've made last several commits in StandardFlowSerializerTest.java trying to detect the problem java.nio.channels.OverlappingFileLockException in org.apache.nifi.controller.scheduling.TestProcessorLifecycle https://issues.apache.org/jira/browse/NIFI-3853?focusedCommentId=16015655=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16015655 and succeed with build. normally should revert this change in my fork... --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user dlukyanov commented on the issue: https://github.com/apache/nifi/pull/1662 @mattyb149 , @joewitt , @alopresto , I know that 1.2.0. released This pull request was not ready ( I need to know your opinion about this pull request - to continue or not. In any case I fixed documentation and nar bundle content. You can try all the cases from pull request description. I built groovyx nar bundle with nifi 1.2.0 https://github.com/dlukyanov/nifi/releases/tag/nifi-groovyx-nar-1.2.0 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user mattyb149 commented on the issue: https://github.com/apache/nifi/pull/1662 Appveyor always fails, no worries. I will take a look at your latest --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user dlukyanov commented on the issue: https://github.com/apache/nifi/pull/1662 seems I fixed all checkstyle issues) but appveyor failed.. what should i do with it? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user mattyb149 commented on the issue: https://github.com/apache/nifi/pull/1662 Also there are still 22 checkstyle violations (mostly lines too long), you can see which files are affected by running "mvn clean install -Pcontrib-check" from your nifi-groovyx-bundle directory. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user mattyb149 commented on the issue: https://github.com/apache/nifi/pull/1662 This NAR won't be added to the NiFi distribution unless you add an entry for it in nifi-assembly/pom.xml, and set its version in the root level pom.xml (see those files for many examples). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor [WIP]
Github user dlukyanov commented on the issue: https://github.com/apache/nifi/pull/1662 @mattyb149, I renamed the processor, created test cases for all mentioned in pull request description. The case [test_sql_01_select.groovy](https://github.com/dlukyanov/nifi/blob/master/nifi-nar-bundles/nifi-groovyx-bundle/nifi-groovyx-processors/src/test/resources/groovy/test_sql_01_select.groovy) dedicated to a new way of controller services usage from groovy script. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user mattyb149 commented on the issue: https://github.com/apache/nifi/pull/1662 I recommend ExecuteGroovyScript as the name, it seems to support the same use case as ExecuteScript but with Groovy-specific bells and whistles. I haven't had a time to review in depth, but will soon. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user alopresto commented on the issue: https://github.com/apache/nifi/pull/1662 @dlukyanov That is not your fault -- @mattyb149 is fixing it in [NIFI-3718](https://issues.apache.org/jira/browse/NIFI-3718). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user dlukyanov commented on the issue: https://github.com/apache/nifi/pull/1662 Can somebody help me? Avro tests fails all the time and I did not touch it. Failed tests: TestAvroRecordReader.testLogicalTypes:109 expected:<2017-04-0[4]> but was:<2017-04-0[5]> --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user dlukyanov commented on the issue: https://github.com/apache/nifi/pull/1662 @joewitt , about naming: CallGroovy would be fine? Actually it's another implementation of groovy script. And the Script processor is just a Script - and not CallScript. I wanted to point that it's extended groovy script.. What do you think about the name? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1662: NIFI-3688 Extended Groovy Nifi Processor
Github user joewitt commented on the issue: https://github.com/apache/nifi/pull/1662 The commit which removed the properties/skip javadoc/src does need to be in the nar bundle itself. We dont want src/javadocs for the nar bundle. Also, the processor needs a name that follows the pattern of VerbSubject instead of GroovyX. We will definitely need unit tests for this. There are references to Oracle things in there or at least in the screenshot. The LICENSE and NOTICE appears to have a lot of copy/paste from other nars. Are all those dependencies really there? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
