dsmiley commented on a change in pull request #1972:
URL: https://github.com/apache/lucene-solr/pull/1972#discussion_r523776751
##########
File path: gradle/solr/packaging.gradle
##########
@@ -76,7 +76,8 @@ configure(allprojects.findAll {project ->
project.path.startsWith(":solr:contrib
return true
}
}
- return externalLibs - configurations.solrPlatformLibs
+ // libExt has logging libs, which we don't want. Lets users decide
what they want.
+ return externalLibs - configurations.solrPlatformLibs -
project(':solr:server').configurations.getByName('libExt')
Review comment:
It might be considered hacky to reference a configuration of a specific
sub-project. Maybe libExt could be declared more top-level and be called
"loggingLibs" or something.
##########
File path: solr/contrib/prometheus-exporter/bin/solr-exporter
##########
@@ -123,8 +111,6 @@ else
GC_TUNE="$GC_TUNE"
fi
-EXTRA_JVM_ARGUMENTS="-Dlog4j.configurationFile=file:"$BASEDIR"/../../server/resources/log4j2-console.xml"
Review comment:
Instead, /conf is put on the classpath, and thus "log4j2.xml" is loaded
automatically based on Log4j2's automatic resolution.
##########
File path: solr/contrib/prometheus-exporter/build.gradle
##########
@@ -31,17 +30,57 @@ dependencies {
exclude group: "org.jruby.joni", module: "joni"
})
implementation ('net.sourceforge.argparse4j:argparse4j')
+ implementation ('com.github.ben-manes.caffeine:caffeine', {
+ exclude group: "org.checkerframework", module: "checker-qual"
+ exclude group: "com.google.errorprone", module: "error_prone_annotations"
+ })
- testImplementation ('org.apache.httpcomponents:httpcore')
- testImplementation ('org.eclipse.jetty:jetty-servlet')
+ runtimeOnly 'org.apache.logging.log4j:log4j-api'
Review comment:
Logging dependencies in Java is a pain. In a separate issue/PR, we
should explore this approach:
https://blog.gradle.org/addressing-logging-complexity-capabilities
##########
File path:
solr/contrib/prometheus-exporter/src/java/org/apache/solr/prometheus/exporter/SolrExporter.java
##########
@@ -68,7 +66,7 @@
private static final String[] ARG_CONFIG_FLAGS = {"-f", "--config-file"};
private static final String ARG_CONFIG_METAVAR = "CONFIG";
private static final String ARG_CONFIG_DEST = "configFile";
- private static final String ARG_CONFIG_DEFAULT = "solr-exporter-config.xml";
+ private static final String ARG_CONFIG_DEFAULT =
"conf/solr-exporter-config.xml";
Review comment:
@HoustonPutman you removed the conf/ and I'm putting it back. I'm not
sure why it worked with it gone. It may be related to other changes in this PR
that it's needed again. I tested that this works here both with `gw run` and
via executing normally from the built assembly. I didn't test Docker yet.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
