[GitHub] nifi pull request: NIFI-1905 enabled ExecuteProcess to terminate p...
Github user asfgit closed the pull request at: https://github.com/apache/nifi/pull/456 --- 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-1905 enabled ExecuteProcess to terminate p...
Github user aperepel commented on the pull request: https://github.com/apache/nifi/pull/456#issuecomment-220795892 LGTM, +1 on this one :) --- 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-1905 enabled ExecuteProcess to terminate p...
Github user olegz commented on the pull request: https://github.com/apache/nifi/pull/456#issuecomment-220795133 @aperepel all squashed and pushed, including the change from WARN to INFO for log message. Let me know if there is anything else. --- 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-1905 enabled ExecuteProcess to terminate p...
Github user aperepel commented on a diff in the pull request:
https://github.com/apache/nifi/pull/456#discussion_r64138418
--- Diff:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ExecuteProcess.java
---
@@ -187,7 +187,7 @@ public void shutdownExecutor() {
executor.shutdown();
} finally {
if (this.externalProcess.isAlive()) {
-this.getLogger().warn("Interrupting external process since
it did not finished at the time stop Processor was initiated.");
+this.getLogger().warn("Process hasn't terminated, forcing
the interrupt");
--- End diff --
Do we want to file a WARN bulleting? It shows up in the UI. E.g. is there a
good way at all to kill a process which doesn't have any external interrupt
handles exposed? Lower to INFO maybe?
---
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-1905 enabled ExecuteProcess to terminate p...
Github user olegz commented on a diff in the pull request:
https://github.com/apache/nifi/pull/456#discussion_r64044369
--- Diff:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ExecuteProcess.java
---
@@ -181,7 +183,14 @@ public Thread newThread(final Runnable r) {
@OnUnscheduled
public void shutdownExecutor() {
-executor.shutdown();
+try {
+executor.shutdown();
+} finally {
+if (this.externalProcess.isAlive()) {
+this.getLogger().warn("Interrupting external process since
it did not finished at the time stop Processor was initiated.");
--- End diff --
_Process hasn't terminated, forcing the interrupt._ - that is what I am
putting in now since we are actually issuing a request to stop something that
has not terminated by itself yet. So, I've removed "on request".
---
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-1905 enabled ExecuteProcess to terminate p...
Github user olegz commented on a diff in the pull request:
https://github.com/apache/nifi/pull/456#discussion_r64043923
--- Diff:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ExecuteProcess.java
---
@@ -181,7 +183,14 @@ public Thread newThread(final Runnable r) {
@OnUnscheduled
public void shutdownExecutor() {
-executor.shutdown();
+try {
+executor.shutdown();
+} finally {
+if (this.externalProcess.isAlive()) {
+this.getLogger().warn("Interrupting external process since
it did not finished at the time stop Processor was initiated.");
--- End diff --
Fair, i'll change. Let me know when you get a chance to test it in your
environment.
---
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-1905 enabled ExecuteProcess to terminate p...
Github user aperepel commented on a diff in the pull request:
https://github.com/apache/nifi/pull/456#discussion_r64040583
--- Diff:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ExecuteProcess.java
---
@@ -181,7 +183,14 @@ public Thread newThread(final Runnable r) {
@OnUnscheduled
public void shutdownExecutor() {
-executor.shutdown();
+try {
+executor.shutdown();
+} finally {
+if (this.externalProcess.isAlive()) {
+this.getLogger().warn("Interrupting external process since
it did not finished at the time stop Processor was initiated.");
--- End diff --
Check for typos and wording, maybe simplify it? How about 'Process hasn't
terminated on request, forcing the interrupt'?
---
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-1905 enabled ExecuteProcess to terminate p...
GitHub user olegz opened a pull request: https://github.com/apache/nifi/pull/456 NIFI-1905 enabled ExecuteProcess to terminate process You can merge this pull request into a Git repository by running: $ git pull https://github.com/olegz/nifi NIFI-1905 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/456.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 #456 commit 7924933b892d3f6b59741389b53ca50c7781e728 Author: Oleg Zhurakousky Date: 2016-05-20T13:05:44Z NIFI-1905 enabled ExecuteProcess to terminate process --- 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. ---
