You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by PuspenduBanerjee <gi...@git.apache.org> on 2016/02/13 04:27:54 UTC

[GitHub] nifi pull request: Enhancement for nifi-1481 [ nifi.sh env]

GitHub user PuspenduBanerjee opened a pull request:

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

    Enhancement for nifi-1481 [ nifi.sh env]

    Initial working version of nifi-1481 , with plenty of room for enhancement.

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

    $ git pull https://github.com/PuspenduBanerjee/nifi pb_nifi-1481

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

    https://github.com/apache/nifi/pull/218.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 #218
    
----
commit c68664c327f4f5163610343165d28667b6bbcb28
Author: puspendu.banerjee@gmail.com <pu...@gmail.com>
Date:   2016-02-10T07:17:00Z

    Fix-1481 support for ./nifi.sh env

commit ec6dc8632e5d72739bd1741079993ca662b4306c
Author: puspendu.banerjee@gmail.com <pu...@gmail.com>
Date:   2016-02-13T03:25:22Z

    Removed compile time Dependency on tools.jar/classes.jar. Added better detection of toos.jar/classes.jar with more meaningful message.

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by PuspenduBanerjee <gi...@git.apache.org>.
Github user PuspenduBanerjee commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-195231594
  
    @apiri For now I have no way to check Windows or Cygwin as I do not have any MS windows installation, total linux guy and on vacation to my homeland, so can't even borrow a windows PC from a colleague. . Please help to find out someone with Windows.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/218#discussion_r55789451
  
    --- Diff: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java ---
    @@ -546,6 +552,31 @@ public void status() throws IOException {
             }
         }
     
    +    public void env(){
    +        final Logger logger = cmdLogger;
    +        final Status status = getStatus(logger);
    +        if(status.getPid() == null){
    +            logger.info("Apache NiFi is not running");
    +            return;
    +        }
    +        try{
    +        final Class<?> virtualMachineClass=Class.forName("com.sun.tools.attach.VirtualMachine");
    --- End diff --
    
    Formatting:  try block should be indented


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-196102613
  
    @trkurc I think it might be a fair concession to have both this and Windows punted. Maybe we just roll in the check piggybacking off of the already existing $cygwin and figure out how we can improve later?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-195034105
  
    reviewing


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by PuspenduBanerjee <gi...@git.apache.org>.
Github user PuspenduBanerjee commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-186010907
  
    @trkurc Please review once you get a chance.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-196092752
  
    @apiri - cygwin is proving to be a challenge, and not necessarily due to the changes in this patch. 
    
    On my setup, the ':' separator on this line seems to break things (I suspect because my java is expecting ';' separators for things on the classpath)
    
    See:
    https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/bin/nifi.sh#L192


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/218#discussion_r55790433
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/bin/nifi.sh ---
    @@ -118,6 +118,12 @@ locateJava() {
                 fi
             fi
         fi
    +    [ "x${TOOLS_JAR}" =  "x" ] && [ -n "${JAVA_HOME}" ] && TOOLS_JAR=$(find "${JAVA_HOME}" -name "tools.jar")
    +    [ "x${TOOLS_JAR}" =  "x" ] && TOOLS_JAR=$(find "${JAVA_HOME}" -name "classes.jar")
    +    if ["x${TOOLS_JAR}" =  "x" ]; then
    --- End diff --
    
    I think my preference would be to opt to only display this if we are using the env command.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-195862019
  
    (See the section labeled "Processing quotation marks ". ). So bizarre.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by PuspenduBanerjee <gi...@git.apache.org>.
Github user PuspenduBanerjee commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-195441150
  
    @trkurc Appreciate the Spirit.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-196564618
  
    So, after discussion with @apiri, we removed the env batch script because it did not work due to limitations of bootstrap, and reworked nifi.sh to build the classpath for Bootstrap to work with cygwin and !cygwin and only include TOOLS_JAR if set.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by jvwing <gi...@git.apache.org>.
Github user jvwing commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-195858721
  
    I have a [suggested fix/hack for dealing with spaces in the Java path in env-nifi.bat](https://github.com/jvwing/nifi/commit/2094080675d0d976fd4cd883d9f97cd10994ba69)).  The fix converts the TOOLS_JAR path into a short 8.3 path without spaces, so the eventual call to NiFi has only one quoted parameter.  The expanded TOOLS_JAR path was not working due to spaces, and two quoted paths make the whole thing blow up for reasons I do not comprehend.
    
    However, this was not enough to get the env feature running on Windows 10.  Running it outputs 
    
    ```
    17:58:32.959 [main] DEBUG o.a.n.b.NotificationServiceManager - Found 0 service elements
    17:58:32.963 [main] INFO  o.a.n.b.NotificationServiceManager - Successfully loaded the following 0 services: []
    17:58:32.965 [main] INFO  org.apache.nifi.bootstrap.RunNiFi - Registered no Notification Services for Notification Type NIFI_STARTED
    17:58:32.987 [main] INFO  org.apache.nifi.bootstrap.RunNiFi - Registered no Notification Services for Notification Type NIFI_STOPPED
    17:58:32.989 [main] INFO  org.apache.nifi.bootstrap.RunNiFi - Registered no Notification Services for Notification Type NIFI_DIED
    17:58:33.012 [main] DEBUG org.apache.nifi.bootstrap.Command - Status File: bin\nifi.pid
    17:58:33.013 [main] DEBUG org.apache.nifi.bootstrap.Command - Status File: bin\nifi.pid
    17:58:33.040 [main] DEBUG org.apache.nifi.bootstrap.Command - Properties: {port=40930}
    17:58:33.041 [main] DEBUG org.apache.nifi.bootstrap.Command - Pinging 40930
    17:58:33.078 [main] DEBUG org.apache.nifi.bootstrap.Command - Sent PING command
    17:58:33.080 [main] DEBUG org.apache.nifi.bootstrap.Command - PING response: PING
    17:58:33.081 [main] INFO  org.apache.nifi.bootstrap.Command - Apache NiFi is not running
    ```
    
    Where the last line "Apache NiFi is not running" matches RunNiFi.java line 559, where the env() method quits after not finding a PID.  Yes, NiFi was running when I tested this.  In comparison, the `status-nifi.bat` outputs
    
    ```
    17:49:49.660 [main] DEBUG o.a.n.b.NotificationServiceManager - Found 0 service elements
    17:49:49.664 [main] INFO  o.a.n.b.NotificationServiceManager - Successfully loaded the following 0 services: []
    17:49:49.666 [main] INFO  org.apache.nifi.bootstrap.RunNiFi - Registered no Notification Services for Notification Type NIFI_STARTED
    17:49:49.690 [main] INFO  org.apache.nifi.bootstrap.RunNiFi - Registered no Notification Services for Notification Type NIFI_STOPPED
    17:49:49.691 [main] INFO  org.apache.nifi.bootstrap.RunNiFi - Registered no Notification Services for Notification Type NIFI_DIED
    17:49:49.719 [main] DEBUG org.apache.nifi.bootstrap.Command - Status File: bin\nifi.pid
    17:49:49.723 [main] DEBUG org.apache.nifi.bootstrap.Command - Status File: bin\nifi.pid
    17:49:49.749 [main] DEBUG org.apache.nifi.bootstrap.Command - Properties: {port=40930}
    17:49:49.752 [main] DEBUG org.apache.nifi.bootstrap.Command - Pinging 40930
    17:49:49.798 [main] DEBUG org.apache.nifi.bootstrap.Command - Sent PING command
    17:49:49.801 [main] DEBUG org.apache.nifi.bootstrap.Command - PING response: PING
    17:49:49.804 [main] INFO  org.apache.nifi.bootstrap.Command - Apache NiFi is currently running, listening to Bootstrap on port 40930, PID=unknkown
    ```
    
    Which recognizes that NiFi is running based on `isRespondingToPing()` (line 532), and describes the PID as "unknkown".  Since it looks like you use the PID in the env method, I'm not immediately sure what to suggest next.  It might be possible to work out an "if Windows, do this to get the PID" flow.



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-196104511
  
    @apiri: how would you think it best to review the changes I made based on your reviews? Another PR? A patch on the ticket?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-195383477
  
    Sounds good, @trkurc.  Just personally ran out of steam digging into it last evening.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/218#discussion_r55791913
  
    --- Diff: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java ---
    @@ -546,6 +552,31 @@ public void status() throws IOException {
             }
         }
     
    +    public void env(){
    +        final Logger logger = cmdLogger;
    +        final Status status = getStatus(logger);
    +        if(status.getPid() == null){
    +            logger.info("Apache NiFi is not running");
    +            return;
    +        }
    +        try{
    +        final Class<?> virtualMachineClass=Class.forName("com.sun.tools.attach.VirtualMachine");
    +        Method attachMethod=virtualMachineClass.getMethod("attach", String.class);
    +        Object virtualMachine=attachMethod.invoke(null, status.getPid());
    +        Method getSystemPropertiesMethod=virtualMachine.getClass().getMethod("getSystemProperties");
    +        final Properties sysProps=(Properties)getSystemPropertiesMethod.invoke(virtualMachine);
    +        for(Entry<Object, Object> syspropEntry: sysProps.entrySet()){
    +            logger.info(syspropEntry.getKey().toString() + " = " +syspropEntry.getValue().toString());
    +        }
    +        Method detachMethod=virtualMachineClass.getDeclaredMethod("detach");
    +        detachMethod.invoke(virtualMachine);
    +        }catch(ClassNotFoundException cnfe){
    +            logger.error("Seems tools.jar(Linux / Windows JDK) or classes.jar(Mac OS) is not available in classpath", cnfe);
    +        }catch(Throwable t){
    +            throw new RuntimeException(t);
    --- End diff --
    
    Not sure the stacktrace provides much information worthwhile to the user.  Would opt to just leave the above log error and remove this.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by PuspenduBanerjee <gi...@git.apache.org>.
Github user PuspenduBanerjee commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-187766997
  
    @trkurc Any chance to review out of your busy schedule yet?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/218#discussion_r55791387
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/bin/env-nifi.bat ---
    @@ -0,0 +1,58 @@
    +@echo off
    +rem
    +rem    Licensed to the Apache Software Foundation (ASF) under one or more
    +rem    contributor license agreements.  See the NOTICE file distributed with
    +rem    this work for additional information regarding copyright ownership.
    +rem    The ASF licenses this file to You under the Apache License, Version 2.0
    +rem    (the "License"); you may not use this file except in compliance with
    +rem    the License.  You may obtain a copy of the License at
    +rem
    +rem       http://www.apache.org/licenses/LICENSE-2.0
    +rem
    +rem    Unless required by applicable law or agreed to in writing, software
    +rem    distributed under the License is distributed on an "AS IS" BASIS,
    +rem    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +rem    See the License for the specific language governing permissions and
    +rem    limitations under the License.
    +rem
    +
    +rem Use JAVA_HOME if it's set; otherwise, just use java
    +
    +if "%JAVA_HOME%" == "" goto noJavaHome
    +if not exist "%JAVA_HOME%\bin\java.exe" goto noJavaHome
    +set JAVA_EXE=%JAVA_HOME%\bin\java.exe
    --- End diff --
    
    Have to be careful with JAVA_HOME throughout as it is highly likely folks using Windows will have this located in a default location with spaces.  This caused troubles at points throughout.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-195861254
  
    push my changes up to a fork, in case anyone was interested (https://github.com/trkurc/nifi/commit/5ebf228a9ba5c5ce9f12189988920f94923e6210 from https://github.com/trkurc/nifi/commits/NIFI-1481)


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/218#discussion_r55790712
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/bin/nifi.sh ---
    @@ -118,6 +118,12 @@ locateJava() {
                 fi
             fi
         fi
    +    [ "x${TOOLS_JAR}" =  "x" ] && [ -n "${JAVA_HOME}" ] && TOOLS_JAR=$(find "${JAVA_HOME}" -name "tools.jar")
    --- End diff --
    
    This was okay on OS X where my Java home was absolute, but not okay on Linux where this was a symlink.  Quick win here would be to use the -H of find


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-195861132
  
    I discovered the same issue with the pid on windows 10. My fix was to use quotes a bit differently on the cmd.exe line (cmd.exe /C ""%JAVA_EXE%" %JAVA_PARAMS% %BOOTSTRAP_ACTION%" ) and (set JAVA_PARAMS=-cp "%TOOLS_JAR%";%CONF_DIR%;%LIB_DIR%\* -Xms12m -Xmx24m %JAVA_ARGS% org.apache.nifi.bootstrap.RunNiFi)
    



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/218#discussion_r55791948
  
    --- Diff: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java ---
    @@ -546,6 +552,31 @@ public void status() throws IOException {
             }
         }
     
    +    public void env(){
    +        final Logger logger = cmdLogger;
    +        final Status status = getStatus(logger);
    +        if(status.getPid() == null){
    +            logger.info("Apache NiFi is not running");
    +            return;
    +        }
    +        try{
    +        final Class<?> virtualMachineClass=Class.forName("com.sun.tools.attach.VirtualMachine");
    +        Method attachMethod=virtualMachineClass.getMethod("attach", String.class);
    +        Object virtualMachine=attachMethod.invoke(null, status.getPid());
    +        Method getSystemPropertiesMethod=virtualMachine.getClass().getMethod("getSystemProperties");
    +        final Properties sysProps=(Properties)getSystemPropertiesMethod.invoke(virtualMachine);
    +        for(Entry<Object, Object> syspropEntry: sysProps.entrySet()){
    +            logger.info(syspropEntry.getKey().toString() + " = " +syspropEntry.getValue().toString());
    +        }
    +        Method detachMethod=virtualMachineClass.getDeclaredMethod("detach");
    +        detachMethod.invoke(virtualMachine);
    --- End diff --
    
    detaching should likely happen in a finally block


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by PuspenduBanerjee <gi...@git.apache.org>.
Github user PuspenduBanerjee commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-195862566
  
    @trkurc  Oh Man! Thanks for your guidance. Any chance of using power-shell in future?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by PuspenduBanerjee <gi...@git.apache.org>.
Github user PuspenduBanerjee commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-183624627
  
    @apiri Can you please review 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-196400346
  
    @apiri - I tried last night to get the pid to get Windows to work. I could not. I say we shelve the env batch script (remove it for now from the patch set). 
    
    On cygwin does "nifi.sh run" or "nifi.sh status" work at all for you without these patches? It did not for me. What were you proposing to fix cygwin? Get run and start working, then try to get env working 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-196384976
  
    @trkurc What is the final verdict on handling of Windows and Cygwin?  Are we punting those for now? If so, would like to provide handling for env with cygwin in nifi.sh and removal of the .bat for now. Happy to do these myself, but wanted to avoid another level of indirection.  Let me know what works.
    
    Otherwise, your branch as presented looks good and works as anticipated.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-195861175
  
    The pid is not being recorded in the nifi.pid file after I run RunNiFi with start or run, haven't been able to diagnose why yet. @markap14 may have an idea.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by PuspenduBanerjee <gi...@git.apache.org>.
Github user PuspenduBanerjee commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-195861516
  
    @trkurc  isn't `cmd.exe /C ""%JAVA_EXE%" %JAVA_PARAMS% %BOOTSTRAP_ACTION%" ` equivalent to `cmd.exe /C %JAVA_EXE%" %JAVA_PARAMS% %BOOTSTRAP_ACTION%" `


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-196365140
  
    @trkurc Patch unfortunately is not applying even on a fresh checkout of the 218 PR.  Will checkout your branch and work from there.
    
    Patch issue was:
    
    > $ git am --signoff ~/Downloads/NIFI-1481.001.patch
    Applying: NIFI-1481: Code review fixes
    error: patch failed: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/bin/env-nifi.bat:50
    error: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/bin/env-nifi.bat: patch does not apply
    Patch failed at 0001 NIFI-1481: Code review fixes


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-196080641
  
    After an out of band discussion with @markap14, seems that windows pids might be challenging to get, so maybe we should leave the Windows env batch script out for 0.6.0. Any objections?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-196348408
  
    @trkurc Okay, will apply over the top and review.  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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-195342717
  
    @apiri @PuspenduBanerjee I can take a whack at those windows fixes. 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/218#discussion_r55789241
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/bin/nifi.sh ---
    @@ -118,6 +118,12 @@ locateJava() {
                 fi
             fi
         fi
    +    [ "x${TOOLS_JAR}" =  "x" ] && [ -n "${JAVA_HOME}" ] && TOOLS_JAR=$(find "${JAVA_HOME}" -name "tools.jar")
    +    [ "x${TOOLS_JAR}" =  "x" ] && TOOLS_JAR=$(find "${JAVA_HOME}" -name "classes.jar")
    +    if ["x${TOOLS_JAR}" =  "x" ]; then
    --- End diff --
    
    missed a space after your [ in your if statement.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-196112326
  
    @apiri - I attached a patch to the ticket, which should be the last two commits off my branch https://github.com/trkurc/nifi/commits/NIFI-1481


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/218#discussion_r52849296
  
    --- Diff: nifi-bootstrap/pom.xml ---
    @@ -41,5 +41,12 @@
     			<groupId>org.apache.nifi</groupId>
     			<artifactId>nifi-expression-language</artifactId>
     		</dependency>
    +		<dependency>
    +          <groupId>com.sun</groupId>
    +          <artifactId>tools</artifactId>
    +          <version>1.7.0+</version>
    +          <scope>system</scope>
    +          <systemPath>${java.home}/../lib/tools.jar</systemPath>
    --- End diff --
    
    I don't know if others have had this experience, but whenever tools was pulled in as a dependency this way, it tender to be fragile.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-195209278
  
    @PuspenduBanerjee going to need to investigate a couple of things further.  With JAVA_HOME set, operation worked as anticipated for both OS X and Linux.  Windows had issues with the path declaration.  Quoted those items, but had issues with establishing NIFI_ROOT.  Cygwin also seemed to have 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-195862994
  
    Also, @jvwing I fixed that misspelling of unknown when I was hacking on RunNiFi


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-195861949
  
    @PuspenduBanerjee sadly, no, no it is not. https://technet.microsoft.com/en-us/library/bb490880.aspx


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-196111289
  
    Patch to apply on top of the PR is probably simplest, but am good with whatever is easiest for 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by PuspenduBanerjee <gi...@git.apache.org>.
Github user PuspenduBanerjee commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/218#discussion_r52863379
  
    --- Diff: nifi-bootstrap/pom.xml ---
    @@ -41,5 +41,12 @@
     			<groupId>org.apache.nifi</groupId>
     			<artifactId>nifi-expression-language</artifactId>
     		</dependency>
    +		<dependency>
    +          <groupId>com.sun</groupId>
    +          <artifactId>tools</artifactId>
    +          <version>1.7.0+</version>
    +          <scope>system</scope>
    +          <systemPath>${java.home}/../lib/tools.jar</systemPath>
    --- End diff --
    
    Actually, we can remove that  declaration in pom. tools.jar/classes.jar has only runtime dependency.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by PuspenduBanerjee <gi...@git.apache.org>.
Github user PuspenduBanerjee commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-187999652
  
    @trkurc Thanks. I shall keep it as is & look forward to see it as part of 0.6.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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NiFi-1481 Enhancement[ nifi.sh env]

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/218#issuecomment-187771732
  
    I looked it over and gave it a try, I don't have any substantive comments yet, this was queued behind getting 0.5.1 out. I think this is a good candidate for 0.6.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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---