You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/09/14 13:07:51 UTC
[GitHub] [solr] risdenk commented on a diff in pull request #1011: SOLR-16410: Upgrade build tool versions
risdenk commented on code in PR #1011:
URL: https://github.com/apache/solr/pull/1011#discussion_r970774519
##########
solr/core/build.gradle:
##########
@@ -81,9 +81,12 @@ dependencies {
implementation 'javax.servlet:javax.servlet-api'
- implementation "org.glassfish.jersey.containers:jersey-container-jetty-http"
+ implementation 'org.glassfish.jersey.containers:jersey-container-jetty-http'
+ permitUnusedDeclared 'org.glassfish.jersey.containers:jersey-container-jetty-http'
implementation 'org.glassfish.jersey.inject:jersey-hk2'
+ permitUnusedDeclared 'org.glassfish.jersey.inject:jersey-hk2'
implementation 'org.glassfish.jersey.media:jersey-media-json-jackson'
+ permitUnusedDeclared 'org.glassfish.jersey.media:jersey-media-json-jackson'
Review Comment:
I'm not 100% convinced of these changes. Do we actually need these dependencies? FYI @gerlowskija
##########
solr/modules/hdfs/src/test/org/apache/solr/hdfs/cloud/HdfsRecoverLeaseTest.java:
##########
@@ -236,13 +206,14 @@ public void close() throws IOException {
}
}
- Set<WriterThread> writerThreads = new HashSet<WriterThread>();
- Set<RecoverThread> recoverThreads = new HashSet<RecoverThread>();
+ Set<WriterThread> writerThreads = new HashSet<>();
+ Set<RecoverThread> recoverThreads = new HashSet<>();
int threadCount = 3;
for (int i = 0; i < threadCount; i++) {
WriterThread wt = new WriterThread(i);
writerThreads.add(wt);
+ // should be wt.start();
Review Comment:
This causes this HDFS test to fail.
##########
solr/modules/hdfs/src/test/org/apache/solr/hdfs/cloud/HdfsRecoverLeaseTest.java:
##########
@@ -148,6 +127,7 @@ public boolean isCallerClosed() {
}
@Test
+ @SuppressWarnings("DoNotCall")
Review Comment:
This is related to the `.start()` but causes tests to fail.
##########
gradle/validation/dependency-analyze.gradle:
##########
@@ -24,7 +24,7 @@ allprojects { prj ->
analyzeClassesDependencies {
warnUsedUndeclared = false // means fail build if UsedUndeclared found
- warnUnusedDeclared = true // means only log warning if UnusedDeclared found
+ warnUnusedDeclared = false // means fail build if UnusedDeclared found
Review Comment:
Now that all the warnings are fixed - this will fail the build if new ones are introduced.
##########
solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientBuilderTest.java:
##########
@@ -160,6 +160,5 @@ public void testProvideExternalClient() throws IOException {
}
// it's external, should be NOT closed when closing CloudSolrClient
verify(http2Client, never()).close();
- http2Client.close();
Review Comment:
This is closing a mock - its not actually needed.
##########
solr/core/build.gradle:
##########
@@ -98,6 +101,7 @@ dependencies {
runtimeOnly "org.apache.lucene:lucene-backward-codecs"
implementation "org.apache.lucene:lucene-codecs"
implementation "org.apache.lucene:lucene-backward-codecs"
+ permitUnusedDeclared "org.apache.lucene:lucene-backward-codecs"
Review Comment:
I think this is due to this being reflection to pull these in?
##########
solr/prometheus-exporter/build.gradle:
##########
@@ -43,7 +43,7 @@ dependencies {
testImplementation project(':solr:core')
testImplementation project(':solr:test-framework')
- testImplementation 'org.apache.lucene:lucene-test-framework'
+ //testImplementation 'org.apache.lucene:lucene-test-framework'
Review Comment:
This can probably be removed. It didn't cause tests to fail being commented out.
##########
solr/modules/jwt-auth/build.gradle:
##########
@@ -19,7 +19,15 @@ apply plugin: 'java-library'
description = 'JWT / OpenID Connect / OAuth2 authentication plugin'
+configurations {
+ constraintsOnly
+ permitTestUnusedDeclared.extendsFrom constraintsOnly
+ implementation.extendsFrom constraintsOnly
+}
Review Comment:
This is a hacky way demonstrated here: https://github.com/gradle-dependency-analyze/gradle-dependency-analyze/issues/108 - I need to go back and add a comment.
##########
solr/modules/clustering/build.gradle:
##########
@@ -29,7 +29,7 @@ dependencies {
implementation 'org.slf4j:slf4j-api'
testImplementation project(':solr:test-framework')
- testImplementation 'org.apache.lucene:lucene-test-framework'
+ //testImplementation 'org.apache.lucene:lucene-test-framework'
Review Comment:
This doesn't cause these tests to fail so can straight remove it looks like.
##########
solr/modules/hdfs/src/test/org/apache/solr/hdfs/cloud/HdfsRecoverLeaseTest.java:
##########
@@ -255,6 +226,7 @@ public void close() throws IOException {
for (WriterThread wt : writerThreads) {
RecoverThread rt = new RecoverThread(wt.getFileId());
recoverThreads.add(rt);
+ // should be rt.start();
Review Comment:
This causes this HDFS test to fail.
##########
solr/solrj/build.gradle:
##########
@@ -51,7 +51,7 @@ dependencies {
testImplementation 'org.apache.zookeeper:zookeeper'
permitTestUnusedDeclared 'org.apache.zookeeper:zookeeper'
- testImplementation 'org.apache.lucene:lucene-analysis-common'
+ //testImplementation 'org.apache.lucene:lucene-analysis-common'
Review Comment:
This can probably be removed? This didn't cause tests to fail when commented out.
##########
solr/modules/hdfs/src/test/org/apache/solr/hdfs/cloud/HdfsRecoverLeaseTest.java:
##########
@@ -255,6 +226,7 @@ public void close() throws IOException {
for (WriterThread wt : writerThreads) {
RecoverThread rt = new RecoverThread(wt.getFileId());
recoverThreads.add(rt);
+ // should be rt.start();
Review Comment:
@madrob do you have any ideas here by chance?
##########
solr/modules/hdfs/src/test/org/apache/solr/hdfs/cloud/HdfsRecoverLeaseTest.java:
##########
@@ -236,13 +206,14 @@ public void close() throws IOException {
}
}
- Set<WriterThread> writerThreads = new HashSet<WriterThread>();
- Set<RecoverThread> recoverThreads = new HashSet<RecoverThread>();
+ Set<WriterThread> writerThreads = new HashSet<>();
+ Set<RecoverThread> recoverThreads = new HashSet<>();
int threadCount = 3;
for (int i = 0; i < threadCount; i++) {
WriterThread wt = new WriterThread(i);
writerThreads.add(wt);
+ // should be wt.start();
Review Comment:
@madrob do you have any ideas here by chance?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org