You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2021/05/04 16:50:24 UTC

[GitHub] [drill] cgivre opened a new pull request #2216: CVE fixes

cgivre opened a new pull request #2216:
URL: https://github.com/apache/drill/pull/2216


   # [DRILL-7918](https://issues.apache.org/jira/browse/DRILL-7918): Dependency Updates for CVE
   
   ## Description
   This PR updates several dependencies to address outstanding CVEs.
   
   The updates are:
   
   * Kudu from version 1.3 to 1.7
   * Avatica from version 1.15 to 1.17
   * HBase from 2.2.2 to 2.4.2
   
   
   ## Documentation
   N/A
   
   ## Testing
   (Please describe how this PR has been tested.)
   


-- 
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:
users@infra.apache.org



[GitHub] [drill] vvysotskyi commented on pull request #2216: DRILL-7918: Dependency Updates for CVE

Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on pull request #2216:
URL: https://github.com/apache/drill/pull/2216#issuecomment-833606174


   Yes, I approved it intentionally, since it is completed and the stuff with unit tests will be completed in a separate PR.


-- 
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:
users@infra.apache.org



[GitHub] [drill] cgivre commented on a change in pull request #2216: DRILL-7918: Dependency Updates for CVE

Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2216:
URL: https://github.com/apache/drill/pull/2216#discussion_r626161624



##########
File path: contrib/storage-kudu/pom.xml
##########
@@ -57,7 +57,7 @@
     <dependency>
       <groupId>org.apache.kudu</groupId>
       <artifactId>kudu-client</artifactId>
-      <version>1.3.0</version>
+      <version>1.14.0</version>

Review comment:
       I did build this and tried it out on my local machine, and successfully got it to connect to Kudu, but I didn't have any data in Kudu... so I couldn't try queries.




-- 
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:
users@infra.apache.org



[GitHub] [drill] vvysotskyi commented on a change in pull request #2216: CVE fixes

Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2216:
URL: https://github.com/apache/drill/pull/2216#discussion_r626021016



##########
File path: contrib/storage-kudu/pom.xml
##########
@@ -57,7 +57,7 @@
     <dependency>
       <groupId>org.apache.kudu</groupId>
       <artifactId>kudu-client</artifactId>
-      <version>1.3.0</version>
+      <version>1.14.0</version>

Review comment:
       Could you please confirm that with the update Drill is able to connect to Kudu? Also, there are no enabled unit tests for Kudu, existing ones are disabled and require running Kudu server.
   Could you please update tests to start Kudu using testcontainers, so we will be sure that it is still working correctly?




-- 
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:
users@infra.apache.org



[GitHub] [drill] cgivre commented on pull request #2216: DRILL-7918: Dependency Updates for CVE

Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2216:
URL: https://github.com/apache/drill/pull/2216#issuecomment-833600796


   > +1
   
   Did you mean to approve this?  I haven't submitted the requested updates with improved unit tests for Kudu. 


-- 
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:
users@infra.apache.org



[GitHub] [drill] cgivre commented on a change in pull request #2216: DRILL-7918: Dependency Updates for CVE

Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2216:
URL: https://github.com/apache/drill/pull/2216#discussion_r626160701



##########
File path: contrib/storage-kudu/pom.xml
##########
@@ -57,7 +57,7 @@
     <dependency>
       <groupId>org.apache.kudu</groupId>
       <artifactId>kudu-client</artifactId>
-      <version>1.3.0</version>
+      <version>1.14.0</version>

Review comment:
       > Could you please confirm that with the update Drill is able to connect to Kudu? Also, there are no enabled unit tests for Kudu, existing ones are disabled and require running Kudu server.
   > Could you please update tests to start Kudu using testcontainers, so we will be sure that it is still working correctly?
   
   Sure!  I've never really used Kudu and don't know much about how it works, but I think I can figure out `testcontainers` and there is a docker container for Kudu.  I'll follow the examples in the Splunk plugin.




-- 
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:
users@infra.apache.org



[GitHub] [drill] luocooong merged pull request #2216: DRILL-7918: Dependency Updates for CVE

Posted by GitBox <gi...@apache.org>.
luocooong merged pull request #2216:
URL: https://github.com/apache/drill/pull/2216


   


-- 
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:
users@infra.apache.org



[GitHub] [drill] cgivre commented on a change in pull request #2216: DRILL-7918: Dependency Updates for CVE

Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2216:
URL: https://github.com/apache/drill/pull/2216#discussion_r626875021



##########
File path: contrib/storage-kudu/pom.xml
##########
@@ -57,7 +57,7 @@
     <dependency>
       <groupId>org.apache.kudu</groupId>
       <artifactId>kudu-client</artifactId>
-      <version>1.3.0</version>
+      <version>1.14.0</version>

Review comment:
       > We have two ignored test classes here, and one of them has code that creates tables in Kudu, and another one queries them. Could you please enable and update them to use the Kudu instance from test containers?
   
   The unit tests for this plugin are frankly pretty poor.  I'll figure out `testcontainers` and update.  Do you think I should break that out in a separate PR?  I'd really like to refactor the unit tests for Kudu if we're going to continue supporting it,  and that represents a lot more work than I intended in this PR.  




-- 
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:
users@infra.apache.org



[GitHub] [drill] vvysotskyi commented on a change in pull request #2216: DRILL-7918: Dependency Updates for CVE

Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2216:
URL: https://github.com/apache/drill/pull/2216#discussion_r626846945



##########
File path: contrib/storage-kudu/pom.xml
##########
@@ -57,7 +57,7 @@
     <dependency>
       <groupId>org.apache.kudu</groupId>
       <artifactId>kudu-client</artifactId>
-      <version>1.3.0</version>
+      <version>1.14.0</version>

Review comment:
       We have two ignored test classes here, and one of them has code that creates tables in Kudu, and another one queries them. Could you please enable and update them to use the Kudu instance from test containers?




-- 
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:
users@infra.apache.org