Re: [PR] used EnvUtils for system properties in core [solr]
epugh commented on PR #4629: URL: https://github.com/apache/solr/pull/4629#issuecomment-5083359417 > Eric, you merged this but it wasn't passing spotless checks. Mikhail cleaned up after you in [86bd520](https://github.com/apache/solr/commit/86bd5203ae91e68dcf9b0e3919a75db96b96a61c) I think it was a different PR actually, but yeah, I saw that and thanked him. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] used EnvUtils for system properties in core [solr]
epugh commented on code in PR #4629: URL: https://github.com/apache/solr/pull/4629#discussion_r3652424417 ## changelog/unreleased/PR#4629-envutils-core-properties.yml: ## @@ -0,0 +1,7 @@ +title: Core system properties can now also be configured via environment variables +type: changed Review Comment: Okay... these changelog distinctinos remain difficult to get right. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] used EnvUtils for system properties in core [solr]
dsmiley commented on code in PR #4629: URL: https://github.com/apache/solr/pull/4629#discussion_r3650725672 ## changelog/unreleased/PR#4629-envutils-core-properties.yml: ## @@ -0,0 +1,7 @@ +title: Core system properties can now also be configured via environment variables +type: changed Review Comment: I think "added" reflects what happened, as the PR is an *opt-in* change for a user -- users need to do something to take advantage of this. "changed" is a change to existing users on an upgrade without them lifting a finger. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] used EnvUtils for system properties in core [solr]
dsmiley commented on PR #4629: URL: https://github.com/apache/solr/pull/4629#issuecomment-5079758424 Eric, you merged this but it wasn't passing spotless checks. Mikhail cleaned up after you in 86bd5203ae91e68dcf9b0e3919a75db96b96a61c -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] used EnvUtils for system properties in core [solr]
epugh merged PR #4629: URL: https://github.com/apache/solr/pull/4629 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] used EnvUtils for system properties in core [solr]
epugh commented on PR #4629: URL: https://github.com/apache/solr/pull/4629#issuecomment-4992588926 > sure, added changelog. thanks! it burped ;-). Can you tweak the type! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] used EnvUtils for system properties in core [solr]
iprithv commented on PR #4629: URL: https://github.com/apache/solr/pull/4629#issuecomment-4983449081 sure, added changelog. thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] used EnvUtils for system properties in core [solr]
epugh commented on PR #4629: URL: https://github.com/apache/solr/pull/4629#issuecomment-4980203603 @iprithv can you add a changelog entry? This is a new feature to Solr, that you can set proeprties via environment variables! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] used EnvUtils for system properties in core [solr]
epugh commented on code in PR #4629:
URL: https://github.com/apache/solr/pull/4629#discussion_r3586944216
##
solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java:
##
@@ -316,7 +316,7 @@ private static Path computeSolrHome(ServletContext
servletContext) {
// Now try system property
final String prop = "solr.solr.home";
source = "system property: " + prop;
- home = System.getProperty(prop);
+ home = EnvUtils.getProperty(prop);
Review Comment:
bit nervous on this, but dug in more and looks okay!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] used EnvUtils for system properties in core [solr]
epugh commented on code in PR #4629:
URL: https://github.com/apache/solr/pull/4629#discussion_r3586942147
##
solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java:
##
@@ -241,7 +241,7 @@ private void logWelcomeBanner(Properties props) {
"Solr typically starts with \"-XX:+CrashOnOutOfMemoryError\"
that will crash on any OutOfMemoryError exception. "
+ "Unable to get the specific file due to an exception."
+ "The cause of the OOME will be logged in a crash file in
the logs directory: %s",
- System.getProperty("solr.logs.dir"));
+ EnvUtils.getProperty("solr.logs.dir"));
Review Comment:
bit nervous on this, but dug in more and looks okay.
##
solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java:
##
@@ -241,7 +241,7 @@ private void logWelcomeBanner(Properties props) {
"Solr typically starts with \"-XX:+CrashOnOutOfMemoryError\"
that will crash on any OutOfMemoryError exception. "
+ "Unable to get the specific file due to an exception."
+ "The cause of the OOME will be logged in a crash file in
the logs directory: %s",
- System.getProperty("solr.logs.dir"));
+ EnvUtils.getProperty("solr.logs.dir"));
Review Comment:
bit nervous on this, but dug in more and looks okay.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] used EnvUtils for system properties in core [solr]
iprithv commented on code in PR #4629:
URL: https://github.com/apache/solr/pull/4629#discussion_r3575054756
##
solr/core/src/java/org/apache/solr/cli/ZkCpTool.java:
##
@@ -154,7 +155,7 @@ public void runImpl(CommandLine cli) throws Exception {
if (dstIsZk) {
String solrHome = cli.getOptionValue(SOLR_HOME_OPTION);
if (StrUtils.isNullOrEmpty(solrHome)) {
-solrHome = System.getProperty("solr.home");
+solrHome = EnvUtils.getProperty("solr.home",
EnvUtils.getProperty("solr.solr.home"));
Review Comment:
my bad, I've fixed ZkCpTool to use EnvUtils.getProperty("solr.home") to
preserve the original behaviour. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] used EnvUtils for system properties in core [solr]
epugh commented on code in PR #4629:
URL: https://github.com/apache/solr/pull/4629#discussion_r3566230286
##
solr/core/src/java/org/apache/solr/cli/ZkCpTool.java:
##
@@ -154,7 +155,7 @@ public void runImpl(CommandLine cli) throws Exception {
if (dstIsZk) {
String solrHome = cli.getOptionValue(SOLR_HOME_OPTION);
if (StrUtils.isNullOrEmpty(solrHome)) {
-solrHome = System.getProperty("solr.home");
+solrHome = EnvUtils.getProperty("solr.home",
EnvUtils.getProperty("solr.solr.home"));
Review Comment:
The `solr.solr.home` thing was always a bit werid... But is a "thing".
What prompted this change here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
