-
Notifications
You must be signed in to change notification settings - Fork 75
Conversation
Thanks for opening a pull request! Could you open an issue for this pull request on Github Issues? https://github.com/oap-project/native-sql-engine/issues Then could you also rename commit message and pull request title in the following format?
See also: |
<dependency> | ||
<groupId>org.apache.logging.log4j</groupId> | ||
<artifactId>log4j-core</artifactId> | ||
<version>2.15.0</version> |
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.
bump to 2.16.0?
we may only need log4j-core?
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.
can you update log4j version to 2.16.0
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.
After our testing, the PR will be postponed until Spark update log4j to 2.16.0 or later since there are lots of unit test still need Spark's log4j dependency, we cannot fix those dependencies by ourselves.
@weiting-chen Regarding data source module, it looks even weird that slf4j/log4j dependencies were not excluded by Following is the dependency tree of module The issue is similar to https://issues.apache.org/jira/browse/MNG-6607
|
@zhztheplayer |
We decide to remove the direct usage for log4j and keep the usage for log4j in spark-core for testing unit test. |
What changes were proposed in this pull request?
(Please fill in changes proposed in this fix)
How was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)