DRILL-7918: Dependency Updates for CVE #2216
Conversation
| @@ -57,7 +57,7 @@ | |||
| <dependency> | |||
| <groupId>org.apache.kudu</groupId> | |||
| <artifactId>kudu-client</artifactId> | |||
| <version>1.3.0</version> | |||
| <version>1.14.0</version> | |||
There was a problem hiding this 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?
There was a problem hiding this 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.
There was a problem hiding this 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.
There was a problem hiding this 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?
There was a problem hiding this 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.
Did you mean to approve this? I haven't submitted the requested updates with improved unit tests for Kudu. |
|
Yes, I approved it intentionally, since it is completed and the stuff with unit tests will be completed in a separate PR. |
DRILL-7918: Dependency Updates for CVE
Description
This PR updates several dependencies to address outstanding CVEs.
The updates are:
Documentation
N/A
Testing
(Please describe how this PR has been tested.)