Skip to content

[BUG] Remove duplicate common.message.* from clients:test jar file#12407

Merged
ijuma merged 1 commit intoapache:trunkfrom
peternied:trunk
Aug 10, 2022
Merged

[BUG] Remove duplicate common.message.* from clients:test jar file#12407
ijuma merged 1 commit intoapache:trunkfrom
peternied:trunk

Conversation

@peternied
Copy link
Copy Markdown
Contributor

@peternied peternied commented Jul 14, 2022

When consuming both kafka-client:3.0.1 and kafka-client:3.0.1:test
through maven a hygene tool was detecting multiple instances of the same
class loaded into the classpath.

Verified this change by building locally with a before and after build with ./gradlew clients:publishToMavenLocal, then used beyond compare to verify the contents.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
    • Minor change to existing build process, the java classes was duplicated and unused.
  • Verify test coverage and CI build status
    • There should be no changes in test coverage and CI build status.
  • Verify documentation (including upgrade notes)
    • No documentation updates need to be made

@peternied
Remove duplicate common.message.* from clients:test jar file
f47da73 
When consuming both `kafka-client:3.0.1` and `kafka-client:3.0.1:test`
through maven a hygene tool was detecting multiple instances of the same
class loaded into the classpath.
@peternied peternied mentioned this pull request Jul 14, 2022
Closed
@peternied
Copy link
Copy Markdown
Contributor Author

peternied commented Jul 15, 2022 

[2022-07-14T20ː10ː48.006Z] Still waiting to schedule task
[2022-07-14T20ː10ː48.006Z] ‘arm4’ is offline

CI failures look like they are related to some infrastructure being offline

@peternied
Copy link
Copy Markdown
Contributor Author

peternied commented Jul 25, 2022

@tombentley Can you help me get this PR reviewed?

ijuma
ijuma approved these changes Aug 10, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@ijuma ijuma left a comment

Choose a reason for hiding this comment

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

Good catch, LGTM.

@ijuma ijuma merged commit 22007fb into apache:trunk Aug 10, 2022
@peternied peternied mentioned this pull request Aug 10, 2022
Closed
ijuma added a commit to franz1981/kafka that referenced this pull request Aug 12, 2022
@ijuma
Merge remote-tracking branch 'apache-github/trunk' into KAFKA-13900
9a644f1 
* apache-github/trunk: (447 commits)
  KAFKA-13959: Controller should unfence Broker with busy metadata log (apache#12274)
  KAFKA-10199: Expose read only task from state updater (apache#12497)
  KAFKA-14154; Return NOT_CONTROLLER from AlterPartition if leader is ahead of controller (apache#12506)
  KAFKA-13986; Brokers should include node.id in fetches to metadata quorum (apache#12498)
  KAFKA-14163; Retry compilation after zinc compile cache error (apache#12507)
  Remove duplicate common.message.* from clients:test jar file (apache#12407)
  KAFKA-13060: Replace EasyMock and PowerMock with Mockito in WorkerGroupMemberTest.java (apache#12484)
  Fix the rate window size calculation for edge cases (apache#12184)
  MINOR: Upgrade gradle to 7.5.1 and bump other build/test dependencies (apache#12495)
  KAFKA-14140: Ensure an offline or in-controlled-shutdown replica is not eligible to join ISR in ZK mode (apache#12487)
  KAFKA-14114: Add Metadata Error Related Metrics
  MINOR: BrokerMetadataSnapshotter must avoid exceeding batch size (apache#12486)
  MINOR: Upgrade mockito test dependencies (apache#12460)
  KAFKA-14144:; Compare AlterPartition LeaderAndIsr before fencing partition epoch (apache#12489)
  KAFKA-14134: Replace EasyMock with Mockito for WorkerConnectorTest (apache#12472)
  MINOR: Update scala version in bin scripts to 2.13.8 (apache#12477)
  KAFKA-14104; Add CRC validation when iterating over Metadata Log Records (apache#12457)
  MINOR: add :server-common test dependency to :storage (apache#12488)
  KAFKA-14107: Upgrade Jetty version for CVE fixes (apache#12440)
  KAFKA-14124: improve quorum controller fault handling (apache#12447)
  ...
@cwperks cwperks mentioned this pull request Sep 28, 2022
Closed
3 tasks
@peternied
Copy link
Copy Markdown
Contributor Author

peternied commented Jan 10, 2023

@ijuma Hi - I was checking the latest versions 3.3.1 and I still see the issue that I tried to fix. Does this change need to be picked into one of the release branches, is there a way I can help to get this done?

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jan 11, 2023

This will be part of 3.4.0. Is there a strong reason to backport it to older releases?

@peternied
Copy link
Copy Markdown
Contributor Author

peternied commented Jan 11, 2023

Thanks for the reply, I would like this fix available sooner since we have to disable jar hell checks in our project to work around this issue for nearly 6 months. Do you have an approximant date for 3.4.0 release?

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jan 12, 2023 •
edited
Loading

Likely this month and it would happen before other releases (3.3.2 is imminent so too late for that).

@peternied
Copy link
Copy Markdown
Contributor Author

peternied commented Jan 12, 2023

That sounds good, thanks for letting me know.

peternied added a commit to peternied/security that referenced this pull request Aug 21, 2023
@peternied
Remove jarhell check disablement
16d4602 
JarHell was disabled due to an issue with Kafka where there were
multiple copies of the same class packaged inside of a release and test
distribution jar.  This was resolved with apache/kafka#12407

The Kafka change has been merged, released, and pulled into this
repository so we are removing the workaround to supress jar hell issue
detection.

- Resolves opensearch-project#1938

Signed-off-by: Peter Nied <petern@amazon.com>
@peternied peternied mentioned this pull request Aug 21, 2023
Merged
3 tasks
peternied added a commit to opensearch-project/security that referenced this pull request Aug 22, 2023
@peternied
Enable jarhell check (#3227)
75f529a 
JarHell was disabled due to an issue with Kafka where there were
multiple copies of the same class packaged inside of a release and test
distribution jar. This was resolved with
apache/kafka#12407

The Kafka change has been merged, released, and pulled into this
repository so we are removing the workaround to supress jar hell issue
detection.

Signed-off-by: Peter Nied <petern@amazon.com>
opensearch-trigger-bot Bot pushed a commit to opensearch-project/security that referenced this pull request Aug 22, 2023
@github-actions
Enable jarhell check (#3227)
c98f764 
JarHell was disabled due to an issue with Kafka where there were
multiple copies of the same class packaged inside of a release and test
distribution jar. This was resolved with
apache/kafka#12407

The Kafka change has been merged, released, and pulled into this
repository so we are removing the workaround to supress jar hell issue
detection.

Signed-off-by: Peter Nied <petern@amazon.com>
(cherry picked from commit 75f529a)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ijuma ijuma ijuma approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@peternied @ijuma