You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by YolandaMDavis <gi...@git.apache.org> on 2016/04/28 10:24:54 UTC
[GitHub] nifi pull request: NIFI-1812 nifi-env scripts (linux/windows)
GitHub user YolandaMDavis opened a pull request:
https://github.com/apache/nifi/pull/386
NIFI-1812 nifi-env scripts (linux/windows)
Initial commit for nifi-env scripts. Scripts will provide a centralized method of setting environmental variables for use by other scripts (e.g. nifi.sh). Changes for linux and windows installations are included. Settings included are JAVA_HOME location, nifi log directory and nifi pid directory.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/YolandaMDavis/nifi NIFI-1812
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/386.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 #386
----
commit 094719be012d1fb2613ec4e6eff2bfbe6050a01e
Author: Yolanda M. Davis <yo...@gmail.com>
Date: 2016-04-26T16:16:39Z
NIFI-1812 initial commit for nifi-env script, includes updates to linux/windows scripts, logback changes, RunNifi/Shutdownhook 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1812 nifi-env scripts (linux/windows)
Posted by YolandaMDavis <gi...@git.apache.org>.
Github user YolandaMDavis commented on the pull request:
https://github.com/apache/nifi/pull/386#issuecomment-218157232
@alopresto added changes based on your comments. 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-1812 nifi-env scripts (linux/windows)
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/386#discussion_r62083303
--- Diff: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java ---
@@ -324,27 +324,46 @@ private void registerNotificationServices(final NotificationServiceManager manag
defaultLogger.info("Registered {} Notification Services for Notification Type {}", registered, type);
}
- File getStatusFile() {
+ File getStatusFile() throws IOException{
return getStatusFile(defaultLogger);
}
- public File getStatusFile(final Logger logger) {
+ public File getStatusFile(final Logger logger) throws IOException{
final File confDir = bootstrapConfigFile.getParentFile();
final File nifiHome = confDir.getParentFile();
- final File bin = new File(nifiHome, "bin");
- final File statusFile = new File(bin, "nifi.pid");
- logger.debug("Status File: {}", statusFile);
+ String pidDir = System.getProperty(NIFI_PID_DIR_PROP);
+
+ final File pidFileDir;
+ if(pidDir != null){
+ pidFileDir = new File(pidDir.trim());
+ } else{
+ pidFileDir = new File(nifiHome, DEFAULT_PID_DIR);
+ }
+
+ FileUtils.ensureDirectoryExistAndCanAccess(pidFileDir);
+ final File statusFile = new File(pidFileDir, "nifi.pid");
+ logger.debug("Status File: {}", statusFile);
return statusFile;
}
- public File getLockFile(final Logger logger) {
+ public File getLockFile(final Logger logger) throws IOException{
--- End diff --
This method's logic is identical to the one above with the exception of the file extension. To reduce code duplication, perhaps we could provide a private method with the logic that just accepts the filename, and both could invoke that with a hard-coded value (even better, constants).
---
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-1812 nifi-env scripts (linux/windows)
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/386#discussion_r62084290
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/bin/nifi-env.bat ---
@@ -0,0 +1,29 @@
+@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 The java implementation to use.
+rem set JAVA_HOME="C:java\"
--- End diff --
I understand this is just a comment, but is that path syntax valid?
---
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-1812 nifi-env scripts (linux/windows)
Posted by YolandaMDavis <gi...@git.apache.org>.
Github user YolandaMDavis commented on a diff in the pull request:
https://github.com/apache/nifi/pull/386#discussion_r62165158
--- Diff: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java ---
@@ -324,27 +324,46 @@ private void registerNotificationServices(final NotificationServiceManager manag
defaultLogger.info("Registered {} Notification Services for Notification Type {}", registered, type);
}
- File getStatusFile() {
+ File getStatusFile() throws IOException{
--- End diff --
Default method is used by the ShutdownHook class which is in the same package and allows it to not have to provide a logger. Not sure why the overloaded method is public (was introduced early on in NIFI-145, default method came later) since it appears to only be used by RunNifi. I will research to see if the public method can really because private/protected.
---
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-1812 nifi-env scripts (linux/windows)
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/386#discussion_r62085710
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/bin/nifi.sh ---
@@ -211,9 +211,9 @@ run() {
# run 'start' in the background because the process will continue to run, monitoring NiFi.
# all other commands will terminate quickly so want to just wait for them
if [ "$1" = "start" ]; then
- (cd "${NIFI_HOME}" && ${sudo_cmd_prefix} "${JAVA}" -cp "${BOOTSTRAP_CLASSPATH}" -Xms12m -Xmx24m -Dorg.apache.nifi.bootstrap.config.file="${BOOTSTRAP_CONF}" org.apache.nifi.bootstrap.RunNiFi $@ &)
+ (cd "${NIFI_HOME}" && ${sudo_cmd_prefix} "${JAVA}" -cp "${BOOTSTRAP_CLASSPATH}" -Xms12m -Xmx24m -Dorg.apache.nifi.bootstrap.config.log.dir="${NIFI_LOG_DIR}" -Dorg.apache.nifi.bootstrap.config.pid.dir="${NIFI_PID_DIR}" -Dorg.apache.nifi.bootstrap.config.file="${BOOTSTRAP_CONF}" org.apache.nifi.bootstrap.RunNiFi $@ &)
else
- (cd "${NIFI_HOME}" && ${sudo_cmd_prefix} "${JAVA}" -cp "${BOOTSTRAP_CLASSPATH}" -Xms12m -Xmx24m -Dorg.apache.nifi.bootstrap.config.file="${BOOTSTRAP_CONF}" org.apache.nifi.bootstrap.RunNiFi $@)
+ (cd "${NIFI_HOME}" && ${sudo_cmd_prefix} "${JAVA}" -cp "${BOOTSTRAP_CLASSPATH}" -Xms12m -Xmx24m -Dorg.apache.nifi.bootstrap.config.log.dir="${NIFI_LOG_DIR}" -Dorg.apache.nifi.bootstrap.config.pid.dir="${NIFI_PID_DIR}" -Dorg.apache.nifi.bootstrap.config.file="${BOOTSTRAP_CONF}" org.apache.nifi.bootstrap.RunNiFi $@)
fi
--- End diff --
Since these two commands differ only by the trailing `&` to indicate a background process, can we store the long command in a variable once? This will reduce the effort to update in the future (forgetting to modify both lines).
Something like:
```
RUN_CMD="cd \"${NIFI_HOME}\" && ${sudo_cmd_prefix} \"${JAVA}\" -cp \"${BOOTSTRAP_CLASSPATH}\" -Xms12m -Xmx24m -Dorg.apache.nifi.bootstrap.config.log.dir=\"${NIFI_LOG_DIR}\" -Dorg.apache.nifi.bootstrap.config.pid.dir=\"${NIFI_PID_DIR}\" -Dorg.apache.nifi.bootstrap.config.file=\"${BOOTSTRAP_CONF}\" org.apache.nifi.bootstrap.RunNiFi \"$@\""
if [ "$1" = "start" ]; then
("$RUN_CMD" &)
else
("$RUN_CMD")
fi
```
---
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-1812 nifi-env scripts (linux/windows)
Posted by YolandaMDavis <gi...@git.apache.org>.
Github user YolandaMDavis commented on a diff in the pull request:
https://github.com/apache/nifi/pull/386#discussion_r62165943
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/bin/nifi.sh ---
@@ -211,9 +211,9 @@ run() {
# run 'start' in the background because the process will continue to run, monitoring NiFi.
# all other commands will terminate quickly so want to just wait for them
if [ "$1" = "start" ]; then
- (cd "${NIFI_HOME}" && ${sudo_cmd_prefix} "${JAVA}" -cp "${BOOTSTRAP_CLASSPATH}" -Xms12m -Xmx24m -Dorg.apache.nifi.bootstrap.config.file="${BOOTSTRAP_CONF}" org.apache.nifi.bootstrap.RunNiFi $@ &)
+ (cd "${NIFI_HOME}" && ${sudo_cmd_prefix} "${JAVA}" -cp "${BOOTSTRAP_CLASSPATH}" -Xms12m -Xmx24m -Dorg.apache.nifi.bootstrap.config.log.dir="${NIFI_LOG_DIR}" -Dorg.apache.nifi.bootstrap.config.pid.dir="${NIFI_PID_DIR}" -Dorg.apache.nifi.bootstrap.config.file="${BOOTSTRAP_CONF}" org.apache.nifi.bootstrap.RunNiFi $@ &)
else
- (cd "${NIFI_HOME}" && ${sudo_cmd_prefix} "${JAVA}" -cp "${BOOTSTRAP_CLASSPATH}" -Xms12m -Xmx24m -Dorg.apache.nifi.bootstrap.config.file="${BOOTSTRAP_CONF}" org.apache.nifi.bootstrap.RunNiFi $@)
+ (cd "${NIFI_HOME}" && ${sudo_cmd_prefix} "${JAVA}" -cp "${BOOTSTRAP_CLASSPATH}" -Xms12m -Xmx24m -Dorg.apache.nifi.bootstrap.config.log.dir="${NIFI_LOG_DIR}" -Dorg.apache.nifi.bootstrap.config.pid.dir="${NIFI_PID_DIR}" -Dorg.apache.nifi.bootstrap.config.file="${BOOTSTRAP_CONF}" org.apache.nifi.bootstrap.RunNiFi $@)
fi
--- End diff --
Agreed I'll take care of 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-1812 nifi-env scripts (linux/windows)
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/386
---
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-1812 nifi-env scripts (linux/windows)
Posted by YolandaMDavis <gi...@git.apache.org>.
Github user YolandaMDavis commented on a diff in the pull request:
https://github.com/apache/nifi/pull/386#discussion_r62165580
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/bin/nifi-env.bat ---
@@ -0,0 +1,29 @@
+@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 The java implementation to use.
+rem set JAVA_HOME="C:java\"
--- End diff --
Not at all valid, that's a typo :) . I think what I'll do is leave it blank but have an example directory location in the above comment, since it can vary by windows/java version.
---
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-1812 nifi-env scripts (linux/windows)
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on the pull request:
https://github.com/apache/nifi/pull/386#issuecomment-218323644
Ran locally on Mac OS X 10.11.4 with both `./bin/nifi.sh start`/`stop` and `./bin/nifi.sh run`. Worked and merged. Backporting to 0.x support branch 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-1812 nifi-env scripts (linux/windows)
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/386#discussion_r62082835
--- Diff: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java ---
@@ -324,27 +324,46 @@ private void registerNotificationServices(final NotificationServiceManager manag
defaultLogger.info("Registered {} Notification Services for Notification Type {}", registered, type);
}
- File getStatusFile() {
+ File getStatusFile() throws IOException{
--- End diff --
Any reason this is default scoped but the overloaded method is public?
---
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.
---