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/20 22:19:56 UTC

[GitHub] [solr] risdenk opened a new pull request, #1031: SOLR-16422: Upgrade Apache Zookeeper to 3.8.0

risdenk opened a new pull request, #1031:
URL: https://github.com/apache/solr/pull/1031

   https://issues.apache.org/jira/browse/SOLR-16422


-- 
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


[GitHub] [solr] risdenk commented on a diff in pull request #1031: SOLR-16422: Upgrade Apache Zookeeper to 3.8.0

Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1031:
URL: https://github.com/apache/solr/pull/1031#discussion_r975922884


##########
gradle/hacks/global-exclude-dependencies.gradle:
##########
@@ -40,6 +40,7 @@ allprojects { prj ->
   ]
   configurations.matching { it.name in configNames }.all {
     exclude group: 'log4j', module: 'log4j' // use SLF4j
+    exclude group: "ch.qos.logback" // use SLF4j
     exclude group: 'commons-logging', module: 'commons-logging' // use SLF4j
     exclude group: 'com.google.code.findbugs', module: 'jsr305' // Uses GPL license
     exclude group: 'com.google.code.findbugs', module: 'annotations' // Use Spotbugs Annotations as replacement

Review Comment:
   I had this logback exclude under all the zookeeper stuff - but then logback came under curator and it just snowballed. It made way more sense to add a global exclude.
   
   There MIGHT be a way to do this with some variables in excludes. That would be good to look at separately.



-- 
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


[GitHub] [solr] risdenk commented on a diff in pull request #1031: SOLR-16422: Upgrade Apache Zookeeper to 3.8.0

Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1031:
URL: https://github.com/apache/solr/pull/1031#discussion_r975920723


##########
solr/modules/sql/build.gradle:
##########
@@ -30,18 +30,17 @@ dependencies {
 
   // SQL Parser via Calcite
   implementation ('org.apache.calcite:calcite-core', {

Review Comment:
   @sonatype-lift ignore



-- 
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


[GitHub] [solr] risdenk commented on a diff in pull request #1031: SOLR-16422: Upgrade Apache Zookeeper to 3.8.0

Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1031:
URL: https://github.com/apache/solr/pull/1031#discussion_r975922134


##########
versions.lock:
##########
@@ -268,11 +273,15 @@ org.glassfish.jersey.inject:jersey-hk2:2.35 (1 constraints: de04fe30)
 org.glassfish.jersey.media:jersey-media-json-jackson:2.35 (1 constraints: de04fe30)
 org.hamcrest:hamcrest:2.2 (3 constraints: 7620ce25)
 org.hamcrest:hamcrest-core:2.2 (1 constraints: cc05fe3f)
+org.immutables:value-annotations:2.9.2 (1 constraints: 0f051236)
 org.itadaki:bzip2:0.9.1 (2 constraints: bd0c4b2c)
 org.javassist:javassist:3.25.0-GA (1 constraints: 2a110ef1)
 org.jctools:jctools-core:3.3.0 (1 constraints: 08050336)
 org.jdom:jdom2:2.0.6.1 (1 constraints: be0c371b)
 org.junit:junit-bom:5.9.0 (1 constraints: c7116dde)
+org.locationtech.jts:jts-core:1.19.0 (2 constraints: a31de760)

Review Comment:
   This replaces `com.esri.geometry:esri-geometry-api:2.2.0 (1 constraints: 5c0db22c)`
   
   https://github.com/apache/solr/pull/1031/commits/fafaaa0fd33f70b535a9fdd947e5742af21201d4#diff-14184d8ebd3edfbb9a6a4454c522968d3ade7e606339876f1fa50f768ea8e897L8



-- 
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


[GitHub] [solr] sonatype-lift[bot] commented on a diff in pull request #1031: SOLR-16422: Upgrade Apache Zookeeper to 3.8.0

Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1031:
URL: https://github.com/apache/solr/pull/1031#discussion_r975898989


##########
solr/modules/sql/build.gradle:
##########
@@ -30,18 +30,17 @@ dependencies {
 
   // SQL Parser via Calcite
   implementation ('org.apache.calcite:calcite-core', {

Review Comment:
   *Medium Vulnerability:*
   ### pkg:maven/org.apache.calcite/calcite-core@1.32.0
   0 Critical, 0 High, 1 Medium, 0 Low, 0 None vulnerabilities have been found across 1 dependencies
   
   To see more details about this component, [go to the Sonatype Lift console](https://lift.sonatype.com/results/github.com/apache/solr/01GDEH25DZV81F963PTS4B1SKR?tab=dependencies&component=pkg%3Amaven%2Forg.apache.calcite%2Fcalcite-core%401.32.0)
   
   ---
   
   <details><summary><b>ℹī¸ Learn about @sonatype-lift commands</b></summary>
   
   You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings.
   | **Command** | **Usage** |
   | ------------- | ------------- |
   | `@sonatype-lift ignore` | Leave out the above finding from this PR |
   | `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR |
   | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
   
   **Note:** When talking to LiftBot, you need to **refresh** the page to see its response.
   <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details>
   
   
   
   ---
   
   Was this a good recommendation?
   [ [🙁 Not relevant](https://www.sonatype.com/lift-comment-rating?comment=335075801&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=335075801&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=335075801&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=335075801&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=335075801&lift_comment_rating=5) ]



-- 
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


[GitHub] [solr] risdenk commented on a diff in pull request #1031: SOLR-16422: Upgrade Apache Zookeeper to 3.8.0

Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1031:
URL: https://github.com/apache/solr/pull/1031#discussion_r975920489


##########
versions.lock:
##########
@@ -268,11 +273,15 @@ org.glassfish.jersey.inject:jersey-hk2:2.35 (1 constraints: de04fe30)
 org.glassfish.jersey.media:jersey-media-json-jackson:2.35 (1 constraints: de04fe30)
 org.hamcrest:hamcrest:2.2 (3 constraints: 7620ce25)
 org.hamcrest:hamcrest-core:2.2 (1 constraints: cc05fe3f)
+org.immutables:value-annotations:2.9.2 (1 constraints: 0f051236)
 org.itadaki:bzip2:0.9.1 (2 constraints: bd0c4b2c)
 org.javassist:javassist:3.25.0-GA (1 constraints: 2a110ef1)
 org.jctools:jctools-core:3.3.0 (1 constraints: 08050336)
 org.jdom:jdom2:2.0.6.1 (1 constraints: be0c371b)
 org.junit:junit-bom:5.9.0 (1 constraints: c7116dde)
+org.locationtech.jts:jts-core:1.19.0 (2 constraints: a31de760)

Review Comment:
   JTS was added to support a bunch of functions in Calcite - that we should get for free.
   
   * https://calcite.apache.org/news/2022/09/10/release-1.32.0/
   * https://calcite.apache.org/docs/history.html#v1-32-0
   * https://issues.apache.org/jira/browse/CALCITE-4294
   * https://issues.apache.org/jira/browse/CALCITE-5262



-- 
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


[GitHub] [solr] risdenk commented on a diff in pull request #1031: SOLR-16422: Upgrade Apache Zookeeper to 3.8.0

Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1031:
URL: https://github.com/apache/solr/pull/1031#discussion_r975995527


##########
versions.lock:
##########
@@ -268,11 +273,15 @@ org.glassfish.jersey.inject:jersey-hk2:2.35 (1 constraints: de04fe30)
 org.glassfish.jersey.media:jersey-media-json-jackson:2.35 (1 constraints: de04fe30)
 org.hamcrest:hamcrest:2.2 (3 constraints: 7620ce25)
 org.hamcrest:hamcrest-core:2.2 (1 constraints: cc05fe3f)
+org.immutables:value-annotations:2.9.2 (1 constraints: 0f051236)
 org.itadaki:bzip2:0.9.1 (2 constraints: bd0c4b2c)
 org.javassist:javassist:3.25.0-GA (1 constraints: 2a110ef1)
 org.jctools:jctools-core:3.3.0 (1 constraints: 08050336)
 org.jdom:jdom2:2.0.6.1 (1 constraints: be0c371b)
 org.junit:junit-bom:5.9.0 (1 constraints: c7116dde)
+org.locationtech.jts:jts-core:1.19.0 (2 constraints: a31de760)

Review Comment:
   I'm extremely against removing dependencies all willy nilly just to meet some weird  goal of "dependency bloat" - yes JTS is needed in Calcite - it enables SQL support for geo functions. Just like esri did. The whole work of moving SQL out of core was so that we could have a thinner core - while still maintaining capabilities.
   
   I saw you asked on Calcite if JTS is optional - it wouldn't be in the Maven dependency tree.



-- 
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


[GitHub] [solr] sonatype-lift[bot] commented on a diff in pull request #1031: SOLR-16422: Upgrade Apache Zookeeper to 3.8.0

Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1031:
URL: https://github.com/apache/solr/pull/1031#discussion_r975920736


##########
solr/modules/sql/build.gradle:
##########
@@ -30,18 +30,17 @@ dependencies {
 
   // SQL Parser via Calcite
   implementation ('org.apache.calcite:calcite-core', {

Review Comment:
   I've recorded this as ignored for this pull request.
   If you change your mind, just comment `@sonatype-lift unignore`.



-- 
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


[GitHub] [solr] dsmiley commented on a diff in pull request #1031: SOLR-16422: Upgrade Apache Zookeeper to 3.8.0

Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1031:
URL: https://github.com/apache/solr/pull/1031#discussion_r975883493


##########
gradle/hacks/global-exclude-dependencies.gradle:
##########
@@ -40,6 +40,7 @@ allprojects { prj ->
   ]
   configurations.matching { it.name in configNames }.all {
     exclude group: 'log4j', module: 'log4j' // use SLF4j
+    exclude group: "ch.qos.logback" // use SLF4j
     exclude group: 'commons-logging', module: 'commons-logging' // use SLF4j
     exclude group: 'com.google.code.findbugs', module: 'jsr305' // Uses GPL license
     exclude group: 'com.google.code.findbugs', module: 'annotations' // Use Spotbugs Annotations as replacement

Review Comment:
   Just FYI (not specific to this PR), if you look at our generated POMs, you will see that each dependency has an excludes list of the list above.  This is even if what's excluded isn't even a transitive dependency to where it's attached.  Our system works but it's annoyingly verbose.



##########
versions.lock:
##########
@@ -268,11 +273,15 @@ org.glassfish.jersey.inject:jersey-hk2:2.35 (1 constraints: de04fe30)
 org.glassfish.jersey.media:jersey-media-json-jackson:2.35 (1 constraints: de04fe30)
 org.hamcrest:hamcrest:2.2 (3 constraints: 7620ce25)
 org.hamcrest:hamcrest-core:2.2 (1 constraints: cc05fe3f)
+org.immutables:value-annotations:2.9.2 (1 constraints: 0f051236)
 org.itadaki:bzip2:0.9.1 (2 constraints: bd0c4b2c)
 org.javassist:javassist:3.25.0-GA (1 constraints: 2a110ef1)
 org.jctools:jctools-core:3.3.0 (1 constraints: 08050336)
 org.jdom:jdom2:2.0.6.1 (1 constraints: be0c371b)
 org.junit:junit-bom:5.9.0 (1 constraints: c7116dde)
+org.locationtech.jts:jts-core:1.19.0 (2 constraints: a31de760)

Review Comment:
   Woah; we're adding a dependency on JTS?  Okay; there's a Solr JIRA that debates doing this for Solr-core.
   I wonder if it's actually used, though?  Like is this for features that a Calcite/Solr user is unlikely to use?  I'm not sure honestly.



-- 
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


[GitHub] [solr] risdenk merged pull request #1031: SOLR-16422: Upgrade Apache Zookeeper to 3.8.0

Posted by GitBox <gi...@apache.org>.
risdenk merged PR #1031:
URL: https://github.com/apache/solr/pull/1031


-- 
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


[GitHub] [solr] risdenk commented on a diff in pull request #1031: SOLR-16422: Upgrade Apache Zookeeper to 3.8.0

Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1031:
URL: https://github.com/apache/solr/pull/1031#discussion_r975920489


##########
versions.lock:
##########
@@ -268,11 +273,15 @@ org.glassfish.jersey.inject:jersey-hk2:2.35 (1 constraints: de04fe30)
 org.glassfish.jersey.media:jersey-media-json-jackson:2.35 (1 constraints: de04fe30)
 org.hamcrest:hamcrest:2.2 (3 constraints: 7620ce25)
 org.hamcrest:hamcrest-core:2.2 (1 constraints: cc05fe3f)
+org.immutables:value-annotations:2.9.2 (1 constraints: 0f051236)
 org.itadaki:bzip2:0.9.1 (2 constraints: bd0c4b2c)
 org.javassist:javassist:3.25.0-GA (1 constraints: 2a110ef1)
 org.jctools:jctools-core:3.3.0 (1 constraints: 08050336)
 org.jdom:jdom2:2.0.6.1 (1 constraints: be0c371b)
 org.junit:junit-bom:5.9.0 (1 constraints: c7116dde)
+org.locationtech.jts:jts-core:1.19.0 (2 constraints: a31de760)

Review Comment:
   JTS was added to support a bunch of functions in Calcite - that we should get for free. Like any user of Solr SQL should be able to use the functions.
   
   * https://calcite.apache.org/news/2022/09/10/release-1.32.0/
   * https://calcite.apache.org/docs/history.html#v1-32-0
   * https://issues.apache.org/jira/browse/CALCITE-4294
   * https://issues.apache.org/jira/browse/CALCITE-5262



-- 
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


[GitHub] [solr] dsmiley commented on a diff in pull request #1031: SOLR-16422: Upgrade Apache Zookeeper to 3.8.0

Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1031:
URL: https://github.com/apache/solr/pull/1031#discussion_r975985027


##########
versions.lock:
##########
@@ -268,11 +273,15 @@ org.glassfish.jersey.inject:jersey-hk2:2.35 (1 constraints: de04fe30)
 org.glassfish.jersey.media:jersey-media-json-jackson:2.35 (1 constraints: de04fe30)
 org.hamcrest:hamcrest:2.2 (3 constraints: 7620ce25)
 org.hamcrest:hamcrest-core:2.2 (1 constraints: cc05fe3f)
+org.immutables:value-annotations:2.9.2 (1 constraints: 0f051236)
 org.itadaki:bzip2:0.9.1 (2 constraints: bd0c4b2c)
 org.javassist:javassist:3.25.0-GA (1 constraints: 2a110ef1)
 org.jctools:jctools-core:3.3.0 (1 constraints: 08050336)
 org.jdom:jdom2:2.0.6.1 (1 constraints: be0c371b)
 org.junit:junit-bom:5.9.0 (1 constraints: c7116dde)
+org.locationtech.jts:jts-core:1.19.0 (2 constraints: a31de760)

Review Comment:
   @joel-bernstein do you gather the above features in Calcite would actually be useful to the way Solr integrates with Calcite, or not, and thus this is just unwelcome dependency bloat?  I'm pushing us to consciously think of these things when upgrading libraries that bring in transitive dependencies.



-- 
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


[GitHub] [solr] dsmiley commented on a diff in pull request #1031: SOLR-16422: Upgrade Apache Zookeeper to 3.8.0

Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1031:
URL: https://github.com/apache/solr/pull/1031#discussion_r975992063


##########
gradle/hacks/global-exclude-dependencies.gradle:
##########
@@ -40,6 +40,7 @@ allprojects { prj ->
   ]
   configurations.matching { it.name in configNames }.all {
     exclude group: 'log4j', module: 'log4j' // use SLF4j
+    exclude group: "ch.qos.logback" // use SLF4j
     exclude group: 'commons-logging', module: 'commons-logging' // use SLF4j
     exclude group: 'com.google.code.findbugs', module: 'jsr305' // Uses GPL license
     exclude group: 'com.google.code.findbugs', module: 'annotations' // Use Spotbugs Annotations as replacement

Review Comment:
   The Gradle documentation describes a few approaches we should explore: https://docs.gradle.org/current/userguide/dependency_downgrade_and_exclude.html 



-- 
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