You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "risdenk (via GitHub)" <gi...@apache.org> on 2023/04/03 14:51:15 UTC

[GitHub] [solr] risdenk opened a new pull request, #1535: Update dependencies com.adobe.testing:s3mock-junit4 to v2.11.0, org.springframework.boot:spring-boot to 2.7.10, org.springframework:spring to 5.3.26, and software.amazon.awssdk to 2.19.1

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

   This replaces https://github.com/apache/solr/pull/1517


-- 
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] HoustonPutman commented on pull request #1535: Update dependencies com.adobe.testing:s3mock-junit4 to v2.11.0, org.springframework.boot:spring-boot to 2.7.10, org.springframework:spring to 5.3.26, and software.amazon.awssdk to 2.19.1

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on PR #1535:
URL: https://github.com/apache/solr/pull/1535#issuecomment-1494499904

   😮 yeah for this and Jetty 11, it would be great to move away from the Spring requirement of the adobe s3-mock.


-- 
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 #1535: Update dependencies com.adobe.testing:s3mock-junit4 to v2.11.0, org.springframework.boot:spring-boot to 2.7.10, org.springframework:spring to 5.3.26, and software.amazon.awssdk to 2.19.1

Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk merged PR #1535:
URL: https://github.com/apache/solr/pull/1535


-- 
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] HoustonPutman commented on a diff in pull request #1535: Update dependencies com.adobe.testing:s3mock-junit4 to v2.11.0, org.springframework.boot:spring-boot to 2.7.10, org.springframework:spring to 5.3.26, and software.amazon.awssdk to 2.19.1

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #1535:
URL: https://github.com/apache/solr/pull/1535#discussion_r1156225049


##########
solr/modules/s3-repository/src/test/org/apache/solr/s3/S3PathsTest.java:
##########
@@ -48,7 +48,7 @@ public void testFiles() throws S3Exception {
   public void testDirectory() throws S3Exception {
 
     client.createDirectory("/simple-directory");
-    assertTrue(client.pathExists("/simple-directory"));
+    assertFalse("Dir path needs a trailing slash", client.pathExists("/simple-directory"));

Review Comment:
   So the integration tests work when I changed one line (which I have pushed), so this _should_ be good, though we have seen other strange errors that the integration tests didn't address. But overall I think we should be good here. directories are generally always treated as directories in the actual backup code...



-- 
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 #1535: Update dependencies com.adobe.testing:s3mock-junit4 to v2.11.0, org.springframework.boot:spring-boot to 2.7.10, org.springframework:spring to 5.3.26, and software.amazon.awssdk to 2.19.1

Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1535:
URL: https://github.com/apache/solr/pull/1535#discussion_r1156351092


##########
solr/modules/s3-repository/src/test/org/apache/solr/s3/S3PathsTest.java:
##########
@@ -48,7 +48,7 @@ public void testFiles() throws S3Exception {
   public void testDirectory() throws S3Exception {
 
     client.createDirectory("/simple-directory");
-    assertTrue(client.pathExists("/simple-directory"));
+    assertFalse("Dir path needs a trailing slash", client.pathExists("/simple-directory"));

Review Comment:
   hmmm I don't see the same issue w/ hdfs tests running `./gradlew test -Ptests.filter="@Nightly and not(@awaitsfix)"`



-- 
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 #1535: Update dependencies com.adobe.testing:s3mock-junit4 to v2.11.0, org.springframework.boot:spring-boot to 2.7.10, org.springframework:spring to 5.3.26, and software.amazon.awssdk to 2.19.1

Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1535:
URL: https://github.com/apache/solr/pull/1535#discussion_r1156371709


##########
solr/modules/s3-repository/src/test/org/apache/solr/s3/S3PathsTest.java:
##########
@@ -48,7 +48,7 @@ public void testFiles() throws S3Exception {
   public void testDirectory() throws S3Exception {
 
     client.createDirectory("/simple-directory");
-    assertTrue(client.pathExists("/simple-directory"));
+    assertFalse("Dir path needs a trailing slash", client.pathExists("/simple-directory"));

Review Comment:
   Also https://ci-builds.apache.org/job/Solr/job/Solr-NightlyTests-main/748/ passed w/o issue for HDFS so not sure what you see :/



-- 
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] HoustonPutman commented on a diff in pull request #1535: Update dependencies com.adobe.testing:s3mock-junit4 to v2.11.0, org.springframework.boot:spring-boot to 2.7.10, org.springframework:spring to 5.3.26, and software.amazon.awssdk to 2.19.1

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #1535:
URL: https://github.com/apache/solr/pull/1535#discussion_r1156113644


##########
solr/modules/s3-repository/src/test/org/apache/solr/s3/S3PathsTest.java:
##########
@@ -48,7 +48,7 @@ public void testFiles() throws S3Exception {
   public void testDirectory() throws S3Exception {
 
     client.createDirectory("/simple-directory");
-    assertTrue(client.pathExists("/simple-directory"));
+    assertFalse("Dir path needs a trailing slash", client.pathExists("/simple-directory"));

Review Comment:
   Hmmm so it appears adobe mock no-longer supports this, but I'm pretty sure that S3 supports it... Kind of annoying.
   
   Not sure what this does with regards to test coverage... As long as the integration test works, I imagine we would be fine here....
   
   I'll try to run the nightlies on the module and see how it goes.



-- 
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] HoustonPutman commented on a diff in pull request #1535: Update dependencies com.adobe.testing:s3mock-junit4 to v2.11.0, org.springframework.boot:spring-boot to 2.7.10, org.springframework:spring to 5.3.26, and software.amazon.awssdk to 2.19.1

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #1535:
URL: https://github.com/apache/solr/pull/1535#discussion_r1156273317


##########
solr/modules/s3-repository/src/test/org/apache/solr/s3/S3PathsTest.java:
##########
@@ -48,7 +48,7 @@ public void testFiles() throws S3Exception {
   public void testDirectory() throws S3Exception {
 
     client.createDirectory("/simple-directory");
-    assertTrue(client.pathExists("/simple-directory"));
+    assertFalse("Dir path needs a trailing slash", client.pathExists("/simple-directory"));

Review Comment:
   The nightlies pass across all implementations but hdfs. I don't think the hdfs failures were related to the change (the tests wouldn't even start)



-- 
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 #1535: Update dependencies com.adobe.testing:s3mock-junit4 to v2.11.0, org.springframework.boot:spring-boot to 2.7.10, org.springframework:spring to 5.3.26, and software.amazon.awssdk to 2.19.1

Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1535:
URL: https://github.com/apache/solr/pull/1535#discussion_r1156202715


##########
solr/modules/s3-repository/src/test/org/apache/solr/s3/S3PathsTest.java:
##########
@@ -48,7 +48,7 @@ public void testFiles() throws S3Exception {
   public void testDirectory() throws S3Exception {
 
     client.createDirectory("/simple-directory");
-    assertTrue(client.pathExists("/simple-directory"));
+    assertFalse("Dir path needs a trailing slash", client.pathExists("/simple-directory"));

Review Comment:
   so I haven't looked in detail, but the s3 client did upgrade so maybe this is a client side AWS change?



-- 
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 pull request #1535: Update dependencies com.adobe.testing:s3mock-junit4 to v2.11.0, org.springframework.boot:spring-boot to 2.7.10, org.springframework:spring to 5.3.26, and software.amazon.awssdk to 2.19.1

Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on PR #1535:
URL: https://github.com/apache/solr/pull/1535#issuecomment-1494527600

   @HoustonPutman can you take a look at 468310c4a856b2264e322f4b859d9599c372c406 - it does fix the tests, but I just feel weird adding/removing `/` from directories. I think there were other changes around this in the past?


-- 
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] HoustonPutman commented on a diff in pull request #1535: Update dependencies com.adobe.testing:s3mock-junit4 to v2.11.0, org.springframework.boot:spring-boot to 2.7.10, org.springframework:spring to 5.3.26, and software.amazon.awssdk to 2.19.1

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #1535:
URL: https://github.com/apache/solr/pull/1535#discussion_r1156373694


##########
solr/modules/s3-repository/src/test/org/apache/solr/s3/S3PathsTest.java:
##########
@@ -48,7 +48,7 @@ public void testFiles() throws S3Exception {
   public void testDirectory() throws S3Exception {
 
     client.createDirectory("/simple-directory");
-    assertTrue(client.pathExists("/simple-directory"));
+    assertFalse("Dir path needs a trailing slash", client.pathExists("/simple-directory"));

Review Comment:
   Yeah not sure. Running again looks good



-- 
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 #1535: Update dependencies com.adobe.testing:s3mock-junit4 to v2.11.0, org.springframework.boot:spring-boot to 2.7.10, org.springframework:spring to 5.3.26, and software.amazon.awssdk to 2.19.1

Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1535:
URL: https://github.com/apache/solr/pull/1535#discussion_r1156308451


##########
solr/modules/s3-repository/src/test/org/apache/solr/s3/S3PathsTest.java:
##########
@@ -48,7 +48,7 @@ public void testFiles() throws S3Exception {
   public void testDirectory() throws S3Exception {
 
     client.createDirectory("/simple-directory");
-    assertTrue(client.pathExists("/simple-directory"));
+    assertFalse("Dir path needs a trailing slash", client.pathExists("/simple-directory"));

Review Comment:
   > I don't think the hdfs failures were related to the change (the tests wouldn't even start)
   
   ut oh... I'll take a closer look at this since I just upgrade Hadoop to 3.3.5... I haven't seen any Jenkins failures related to it though.



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