Skip to content

[improve][client] Exclude log4j-slf4j-impl from compile dep in pulsar-client-all#19937

Merged
tisonkun merged 1 commit into
apache:masterfrom
tisonkun:exclude-log4j-impl
Mar 28, 2023
Merged

[improve][client] Exclude log4j-slf4j-impl from compile dep in pulsar-client-all#19937
tisonkun merged 1 commit into
apache:masterfrom
tisonkun:exclude-log4j-impl

Conversation

@tisonkun

Copy link
Copy Markdown
Member

This closes #19484.

The root cause may be apache/bookkeeper@f0988dd#r106236889 and we bump to affected BK version in Pulsar 2.11.

Motivation

Avoid pull in log4j-slf4j-impl in pulsar-client-all.

Modifications

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

…-client-all

Signed-off-by: tison <wander4096@gmail.com>
@tisonkun tisonkun self-assigned this Mar 27, 2023
@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Mar 27, 2023

@eolivelli eolivelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good catch

would you fix it also in bookkeeper ?
there is an ongoing VOTE for 4.16.0

@tisonkun

Copy link
Copy Markdown
Member Author

would you fix it also in bookkeeper ?

I'm glad to do so. But updating dependencies can be tricky. I have a conversation with @shoothzj and he has some ideas to improve the overall situation. I'll wait for his updates.

Anyway, file an issue at apache/bookkeeper#3891

@hezhangjian

Copy link
Copy Markdown
Member

sorry for my late reply. @eolivelli @tisonkun I have submit a PR for review apache/bookkeeper#3892.
I verified the module doesn't contains log4j dependency. And the server package contains it.

@tisonkun

Copy link
Copy Markdown
Member Author

Merging..

After we upgrade to BK 4.16.0, we can revisit this issue and do some cleanups.

@tisonkun tisonkun merged commit d14e43e into apache:master Mar 28, 2023
tisonkun added a commit that referenced this pull request Mar 28, 2023
…-client-all (#19937)

(cherry picked from commit d14e43e)

Ref - #19484

Signed-off-by: tison <wander4096@gmail.com>
@tisonkun tisonkun deleted the exclude-log4j-impl branch March 28, 2023 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] pulsar-client-all 2.11.0 transitively bringing in log4j2 implementation

3 participants