-
Notifications
You must be signed in to change notification settings - Fork 84
AMBARI-26487: Bump commons-io:commons-io from 2.4 to 2.14.0 in /ambari-metrics-common #159
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
base: master
Are you sure you want to change the base?
Conversation
jojochuang
left a comment
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 change by itself is good. I verified it didn't break anything: https://github.com/apache/ambari-metrics/pull/157/commits
But commons-io is 2.1 in ambari-metrics-hadoop-sink ,
2.4 in ambari-metrics-timelineservice. Can we get them updated as well?
Sure, I'd be happy to update the PR. |
|
@0lai0 When upgrading dependencies in Ambari Metrics, extreme caution must be exercised due to version coupling with Hadoop dependencies. These dependencies are packaged into the Hadoop-sink and embedded into Hadoop as a plugin. If the versions conflict with Hadoop's dependency versions, it can directly cause Hadoop to fail to start. |
|
Therefore, the best approach is to quickly deploy a test cluster after upgrading the dependencies. However, the one-click Docker cluster deployment project is not yet available, making testing quite challenging. Previously, upgrading to JDK 17 caused the Hadoop Sink to be compiled with JDK 17, and when embedded into Hadoop as a plugin, the JDK version mismatch led to startup failures. |
|
I see. Thanks for the heads up. Perhaps we should add an integration test module too otherwise these kind of frictions is too much. |
|
We should consider shade the jars to avoid version conflicts. I'd like to study that possibility soon. |
jojochuang
left a comment
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.
+1 from me but let's wait for more validation.
Thank you for your comprehensive explanation, which has greatly clarified the risks and precautions involved in upgrading dependencies for Ambari Metrics. |
What changes were proposed in this pull request?
Bump commons-io:commons-io from 2.4 to 2.14.0 in /ambari-metrics-common
How was this patch tested?
cd ambari-metrics-common
mvn test
mvn dependency:tree -DignoreErrors | grep commons-io
mvn dependency:tree -DignoreErrors