-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove usages of commons-text library lower than 1.10.0 #7850
Conversation
build.gradle
Outdated
@@ -96,7 +96,7 @@ project.ext.externalDependency = [ | |||
'guice': 'com.google.inject:guice:4.2.3', | |||
'guava': 'com.google.guava:guava:27.0.1-jre', | |||
'h2': 'com.h2database:h2:2.1.214', | |||
'hadoopClient': 'org.apache.hadoop:hadoop-client:3.2.4', | |||
'hadoopClient': 'org.apache.hadoop:hadoop-client:3.3.5', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This worries me a bit. Our Spark components are sensitive to this version. I would instead look to control the transitive dependency here too. If there is a newer 3.2.x that would be fine for sure, but I do believe they are not releasing any more minor versions there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no new 3.2.x versions... I put this out to see if the build would pass but it looks like not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iprentic - I took a look this is actually very limited in just the datahub-upgrade
module and that it is only involved with reading parquet, not a full Spark stack. My initial assumption here was wrong. The only thing I noticed is that the the hadoop commons library a few lines down should be matched with this version. So 3.3.4 -> 3.3.5 for both. I've added a common variable to make sure they are alignment.
@iprentic once this is green we can merge! |
Co-authored-by: Indy Prentice <[email protected]> Co-authored-by: David Leifker <[email protected]>
Version not vulnerable to CVE-2022-42889
Checklist