[GitHub] nifi pull request: NIFI-1822: Allow concurrent execution in Execut...

2016-05-14 Thread mattyb149
Github user mattyb149 commented on the pull request:

https://github.com/apache/nifi/pull/387#issuecomment-219265046
  
Thanks much for the corrections/improvements/additions!


---
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 pull request: NIFI-1822: Allow concurrent execution in Execut...

2016-05-14 Thread mattyb149
Github user mattyb149 closed the pull request at:

https://github.com/apache/nifi/pull/387


---
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 pull request: NIFI-1822: Allow concurrent execution in Execut...

2016-05-14 Thread alopresto
Github user alopresto commented on the pull request:

https://github.com/apache/nifi/pull/387#issuecomment-219264620
  
I have opened a new [PR 443](https://github.com/apache/nifi/pull/443) with 
unit tests, fixes for the above issues, and additional changes.  


---
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 pull request: NIFI-1822 ExecuteScript should allow concurrent...

2016-05-14 Thread alopresto
GitHub user alopresto opened a pull request:

https://github.com/apache/nifi/pull/443

NIFI-1822 ExecuteScript should allow concurrent execution

I have added some unit tests and fixed a few minor things in @mattyb149 's 
original contribution ([PR 387](https://github.com/apache/nifi/pull/387)). 

At the time I am opening the PR, I am not able to do a complete build on my 
system because the Apache Maven repositories are unavailable due to a technical 
issue, but all tests in the module with these changes pass, and the checkstyle 
and RAT rules pass. I'll re-run the complete build when the repositories are 
available. 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/alopresto/nifi pr387

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi/pull/443.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #443


commit 23e4f685c3e6a5c83538b7e3e379f1bb1c6002b7
Author: Matt Burgess 
Date:   2016-04-28T13:53:16Z

NIFI-1822: Allow concurrent execution in ExecuteScript

commit fd9120c28bd84a8f88a49798e1290d32c16a024d
Author: Andy LoPresto 
Date:   2016-05-10T23:08:02Z

NIFI-1822 Added unit test skeleton for pooled script processor execution.

commit 8c5ba511283d3ddcbeb56a43240e8640e66a7357
Author: Andy LoPresto 
Date:   2016-05-14T02:03:34Z

NIFI-1822 Added variable max concurrent task field in MockProcessorContext 
because it was previously hardcoded to 1. Changed setNumThreads to 
setMaxConcurrentTasks to maintain naming convention.

commit 8ab0ce59a408f5090c68a954dc1496de275aded1
Author: Andy LoPresto 
Date:   2016-05-14T02:06:17Z

NIFI-1822 Added for loop to instantiate multiple script engines in queue.

commit 4c174c8ca76f5a9e64aa2bd2469ca2966c90b888
Author: Andy LoPresto 
Date:   2016-05-14T02:06:53Z

NIFI-1822 Added debugging messages to script execution.

commit 12bbe82381fb334ff236db6815ada8a277a782e7
Author: Andy LoPresto 
Date:   2016-05-14T02:07:53Z

NIFI-1822 Moved unit test to correct directory.
Added test script resource which generates flowfile and updates attribute 
with current thread.
Added tests for single run, serial run, and pooled run (not complete).

commit bdbc4bab04ae3953087fcfc5192fac19da813e83
Author: Andy LoPresto 
Date:   2016-05-14T02:24:07Z

NIFI-1822 Renamed reference to MockProcessorContext#setNumThreads to 
setMaxConcurrentTasks after refactor.

commit 9264428bd8515aa74b73e0a84b9f46c3cd749400
Author: Andy LoPresto 
Date:   2016-05-15T03:12:02Z

NIFI-1822 Removed debugging log statements for script engine queue size.
Added unit tests demonstrating pooled execution timing and thread usage.

commit cb108cd4376c61b391a5d9d3d269d874bb0f65c5
Author: Andy LoPresto 
Date:   2016-05-15T03:21:15Z

NIFI-1822 Added ASF License to unit test.

commit 7c5acc1ee28b711bb4878371a31ddb267fae79bd
Author: Andy LoPresto 
Date:   2016-05-15T03:37:05Z

NIFI-1822 Removed trailing whitespace to conform with checkstyle rules.

commit 612a7599e240d2378e94c31abf2f2525b45ce9da
Author: Andy LoPresto 
Date:   2016-05-15T03:37:38Z

NIFI-1822 Removed unused variable in unit test.




---
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 pull request: NIFI-1754 Rollback log messages should include ...

2016-05-14 Thread jskora
Github user jskora commented on a diff in the pull request:

https://github.com/apache/nifi/pull/430#discussion_r63279086
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/repository/StandardProcessSession.java
 ---
@@ -175,6 +184,10 @@ public StandardProcessSession(final ProcessContext 
context) {
 this.sessionId = idGenerator.getAndIncrement();
 this.connectableDescription = description;
 
+this.isRollbackCountEnabled = 
NiFiProperties.getInstance().isRollbackCountEnabled();
+this.isRollbackLogUnackFFEnabled = 
NiFiProperties.getInstance().isRollbackLogUnackFFEnabled();
+this.rollbackLogUnackFFMax = 
NiFiProperties.getInstance().getRollbackLogUnackFFMax();
+
--- End diff --

Good point.  So, is the correct handling to have a successful transfer or 
commit drop the attribute?


---
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 pull request: NIFI-1754 Rollback log messages should include ...

2016-05-14 Thread jskora
Github user jskora commented on a diff in the pull request:

https://github.com/apache/nifi/pull/430#discussion_r63279016
  
--- Diff: 
nifi-api/src/main/java/org/apache/nifi/processor/ProcessSession.java ---
@@ -109,6 +109,19 @@
 void rollback(boolean penalize);
 
 /**
+ * Produces a listing of pertinent information about the session's
+ * unacknowledged flowfiles that can be used for logging purposes.  If 
the
+ * session fails for any reason this can help identify problem files 
with
+ * minimal system impact.
+ *
+ * Because of testing dependencies and ot
+ *
+ * @return {@link String} listing pertinent information about the 
session's
+ * unacknowledged flowfiles, primarily intended for logging purposes.
+ */
+String getUnacknowledgedFlowfileInfo();
--- End diff --

The root problem is tracking "Administratively Yielding" messages back to a 
flow file, so it lists the unacknowledged flow files since committed and 
transferred files were already processed.  Looking at where the 
"Administratively Yielding" messages come from in 
[EventDrivenSchedulingAgent](https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/scheduling/EventDrivenSchedulingAgent.java#L337)
 and 
[ContinuallyRunProcessTask](https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/tasks/ContinuallyRunProcessorTask.java#L167),
 the framework doesn't have visibility into the active processor that threw the 
uncaught exception.  This implementation populates the log message without 
taxing the framework or requiring support in the processors, but it can still 
be overridden for customiza
 tion by individual processors.

Providing flow files objects instead of a log message would expose 
internals between framework layers and complicate api and implementation module 
dependencies in the build.  The upper level framework delegates flow file 
handling further down, so this approach similarly delegates the log message 
creation with minimal risk and impact on the framework.


---
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 pull request: NIFI-1754 Rollback log messages should include ...

2016-05-14 Thread jskora
Github user jskora commented on a diff in the pull request:

https://github.com/apache/nifi/pull/430#discussion_r63277496
  
--- Diff: 
nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -21,13 +21,19 @@
 public abstract class AbstractProcessor extends 
AbstractSessionFactoryProcessor {
 
 @Override
+protected void init(ProcessorInitializationContext context) {
+super.init(context);
--- End diff --

I think there was originally some code in the method that got removed, I 
believe I can remove this method.


---
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 pull request: NIFI-1754 Rollback log messages should include ...

2016-05-14 Thread jskora
Github user jskora commented on a diff in the pull request:

https://github.com/apache/nifi/pull/430#discussion_r63277482
  
--- Diff: nifi-api/pom.xml ---
@@ -21,5 +21,12 @@
 0.7.0-SNAPSHOT
 
 nifi-api
-jar
+jar
+
+
+org.apache.nifi
+nifi-properties
+${project.version}
--- End diff --

Makes sense, I can fix that.


---
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 pull request: NIFI-856 Implements experimental ListenLumberja...

2016-05-14 Thread trixpan
Github user trixpan commented on the pull request:

https://github.com/apache/nifi/pull/290#issuecomment-219208792
  
@apiri - Sorry for the delay in putting this together but hopefully all 
comments should be addressed now. 



---
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 pull request: NIFI-856 Implements experimental ListenLumberja...

2016-05-14 Thread trixpan
Github user trixpan commented on a diff in the pull request:

https://github.com/apache/nifi/pull/290#discussion_r63273021
  
--- Diff: 
nifi-nar-bundles/nifi-lumberjack-bundle/nifi-lumberjack-processors/src/test/java/org/apache/nifi/processors/lumberjack/handler/TestLumberjackFrameHandler.java
 ---
@@ -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.
+ */
+package org.apache.nifi.processors.lumberjack.handler;
+
+import org.apache.nifi.logging.ProcessorLog;
+import 
org.apache.nifi.processor.util.listen.dispatcher.AsyncChannelDispatcher;
+import org.apache.nifi.processor.util.listen.event.EventFactory;
+import org.apache.nifi.processor.util.listen.response.ChannelResponder;
+import org.apache.nifi.processor.util.listen.response.ChannelResponse;
+import org.apache.nifi.processors.lumberjack.event.LumberjackEvent;
+import org.apache.nifi.processors.lumberjack.event.LumberjackEventFactory;
+import org.apache.nifi.processors.lumberjack.frame.LumberjackEncoder;
+import org.apache.nifi.processors.lumberjack.frame.LumberjackFrame;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.io.IOException;
+import java.nio.channels.SelectionKey;
+import java.nio.channels.SocketChannel;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+import javax.xml.bind.DatatypeConverter;
+
+
+
+public class TestLumberjackFrameHandler {
+private Charset charset;
+private EventFactory eventFactory;
+private BlockingQueue events;
+private SelectionKey key;
+private AsyncChannelDispatcher dispatcher;
+private LumberjackEncoder encoder;
+
+private ProcessorLog logger;
+
+private LumberjackFrameHandler frameHandler;
+
+@Before
+public void setup() {
+this.charset = StandardCharsets.UTF_8;
+this.eventFactory = new LumberjackEventFactory();
+this.events = new LinkedBlockingQueue<>();
+this.key = Mockito.mock(SelectionKey.class);
+this.dispatcher = Mockito.mock(AsyncChannelDispatcher.class);
+this.logger = Mockito.mock(ProcessorLog.class);
+this.encoder = new LumberjackEncoder();
+
+this.frameHandler = new LumberjackFrameHandler<>(key, charset, 
eventFactory, events, dispatcher, logger);
+}
+
+@Test
+public void testOpen() throws IOException, InterruptedException {
+final LumberjackFrame openFrame = new LumberjackFrame.Builder()
+.version((byte) 0x31)
+.frameType((byte) 0x57)
+.dataSize(0)
+.seqNumber(-1)
+.payload(DatatypeConverter.parseHexBinary("0001"))
+.build();
+
+
+final String sender = "sender1";
+final CapturingChannelResponder responder = new 
CapturingChannelResponder();
+
+// call the handler and verify respond() was called once with once 
response
+frameHandler.handle(openFrame, responder, sender);
+
+// TODO: Implement assertions
--- End diff --

Added a few asserts as well


---
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 pull request: NIFI-856 Implements experimental ListenLumberja...

2016-05-14 Thread trixpan
Github user trixpan commented on a diff in the pull request:

https://github.com/apache/nifi/pull/290#discussion_r63273019
  
--- Diff: 
nifi-nar-bundles/nifi-lumberjack-bundle/nifi-lumberjack-processors/src/test/java/org/apache/nifi/processors/lumberjack/frame/TestLumberjackEncoder.java
 ---
@@ -0,0 +1,23 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.lumberjack.frame;
+
+
+
+public class TestLumberjackEncoder {
+// TODO: Implement
--- End diff --

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.
---