Skip to content

Remove logback core depenency as it's not been used anywhere#21819

Merged
ajaygeorge merged 1 commit intoprestodb:masterfrom
Akanksha-kedia:cl
Feb 13, 2024
Merged

Remove logback core depenency as it's not been used anywhere#21819
ajaygeorge merged 1 commit intoprestodb:masterfrom
Akanksha-kedia:cl

Conversation

@Akanksha-kedia
Copy link
Contributor

@Akanksha-kedia Akanksha-kedia commented Jan 30, 2024

Description

Remove logback core depenency as its not been used anywhere

Motivation and Context

i see vulnerabilities as well:
Direct vulnerabilities:
CVE-2023-6378
CVE-2021-42550

using mvn dependency:tree -Dmaven.wagon.http.ssl.insecure=true -Dmaven.wagon.http.ssl.allowall=true

+- com.facebook.airlift:log:jar:0.207:compile
[INFO] +- com.facebook.airlift:log-manager:jar:0.207:compile
[INFO] | +- org.slf4j:slf4j-jdk14:jar:1.7.25:runtime
[INFO] | | - org.slf4j:slf4j-api:jar:1.7.25:runtime
[INFO] | +- org.slf4j:log4j-over-slf4j:jar:1.7.25:runtime
[INFO] | +- org.slf4j:jcl-over-slf4j:jar:1.7.25:runtime
[INFO] | - ch.qos.logback:logback-core:jar:1.2.3:compile
[INFO] +- com.facebook.airlift:http-server:jar:0.207:compile
[INFO] | +- com.facebook.airlift:http-utils:jar:0.207:compile
[INFO] | +- com.facebook.airlift:security:jar:0.207:compile
[INFO] | +- com.facebook.airlift:stats:jar:0.207:compile
[INFO] | | +- org.hdrhistogram:HdrHistogram:jar:2.1.9:compile
[INFO] | | +- io.airlift:slice:jar:0.34:compile

presto directly is not using the dependency of logback-core

Impact

no impact

Test Plan

Ran test cases

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

== RELEASE NOTE ==
Security Changes
* Remove logback 1.2.3

@Akanksha-kedia Akanksha-kedia requested a review from a team as a code owner January 30, 2024 10:21
@steveburnett
Copy link
Contributor

Similar to my suggestion on PR 21668, if removing this package is appropriate, then - as this is a change to the Presto code - perhaps something other than NO RELEASE NOTE is appropriate.

Perhaps something like the following:

== RELEASE NOTES ==
Security Changes
* Removed logback 1.2.3

Note: If logback 1.2.3 is not the correct name and version of the removed package, change that to the correct name and version of the removed package.

@skairali skairali self-assigned this Jan 30, 2024
Copy link
Member

@skairali skairali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good change from security perspective. We don't need to keep and maintain unused packages

@skairali
Copy link
Member

@Akanksha-kedia can you please add the release notes changes as @steveburnett suggested

@Akanksha-kedia
Copy link
Contributor Author

@steveburnett please review

@Akanksha-kedia
Copy link
Contributor Author

@tdcmeehan please help to review

@steveburnett
Copy link
Contributor

Could you share the result of the test cases you ran in your Test Plan?

@Akanksha-kedia
Copy link
Contributor Author

Could you share the result of the test cases you ran in your Test Plan?

hello, i have ran build with test cases as a part of my test plan and test cases ?

var directory:

image
image

@Akanksha-kedia
Copy link
Contributor Author

@tdcmeehan please help to review @skairali help to merge. @steveburnett

@ajaygeorge
Copy link
Contributor

Couple of small nits

  1. its -> it's in the commit message
  2. Removed -> Remove in Release noes to keep the text in imperative mood.

@Akanksha-kedia Akanksha-kedia changed the title Remove logback core depenency as its not been used anywhere Remove logback core depenency as it's not been used anywhere Feb 13, 2024
@Akanksha-kedia
Copy link
Contributor Author

@ajaygeorge i have done.

@ajaygeorge ajaygeorge merged commit 468de33 into prestodb:master Feb 13, 2024
@wanglinsong wanglinsong mentioned this pull request May 1, 2024
48 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

4 participants