Skip to content
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

[feat][misc] Add Pulsar BOM (Bill of Materials) #21871

Merged
merged 2 commits into from
Jan 11, 2024

Conversation

onobc
Copy link
Contributor

@onobc onobc commented Jan 10, 2024

This commit introduces the pulsar-bom module which fulfills PIP-326 by providing a Maven BOM to ease dependency management.

PIP: PIP-326

Motivation

As described in PIP-326, the purpose of this change is to provide a Maven BOM to ease dependency management for consumers of Pulsar.

Modifications

  • Adds a new pulsar-bom module that provides the Maven BOM
  • The included pom.xml was auto-generated using the provided gen-pulsar-bom.sh script.
  • The script can be run anytime to update the <dependencyManagement> section in the pom.xml w/ the latest published Pulsar modules.

Verifying this change

  • Make sure that the change passes the CI checks.
  • Add automated test app that consumes the BOM (??)

❓This change adds a Maven BOM that consumers of the Pulsar library can use. As such, it is not your typical test approach to verify this. I have verified manually using a local test app. Should I add an automated test/app that consumes the BOM? If so, where would be the best place to add this in the repo?

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 (since this is a new published module, possibly fits this box?)

Documentation

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

Matching PR in forked repository

PR in forked repository: onobc#3

This commit introduces the `pulsar-bom` module which fulfills PIP-326
by providing a Maven BOM to ease dependency management.

The included pom.xml was auto-generated using the
provided `gen-pulsar-bom.sh` script. The script can be run anytime to
update the pom.xml <dependencyManagement> section w/ the latest
published Pulsar modules.

See PIP-326
@merlimat merlimat added this to the 3.3.0 milestone Jan 10, 2024
@merlimat
Copy link
Contributor

@onobc Should we also add a mention in the Java client docs page on how to use the BOM?

@codecov-commenter
Copy link

Codecov Report

Attention: 13 lines in your changes are missing coverage. Please review.

Comparison is base (6560a21) 73.55% compared to head (7c43aea) 73.55%.
Report is 3 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #21871   +/-   ##
=========================================
  Coverage     73.55%   73.55%           
- Complexity    32314    32322    +8     
=========================================
  Files          1858     1859    +1     
  Lines        138201   138281   +80     
  Branches      15150    15156    +6     
=========================================
+ Hits         101652   101712   +60     
- Misses        28665    28681   +16     
- Partials       7884     7888    +4     
Flag Coverage Δ
inttests 24.13% <0.00%> (-0.02%) ⬇️
systests 23.82% <1.11%> (+0.06%) ⬆️
unittests 72.83% <85.55%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...che/bookkeeper/mledger/impl/ManagedLedgerImpl.java 80.90% <100.00%> (ø)
...pache/bookkeeper/mledger/offload/OffloadUtils.java 77.47% <100.00%> (ø)
...dbalance/extensions/ExtensibleLoadManagerImpl.java 78.09% <100.00%> (-1.01%) ⬇️
...xtensions/channel/ServiceUnitStateChannelImpl.java 84.46% <ø> (-0.32%) ⬇️
...alance/extensions/manager/StateChangeListener.java 100.00% <100.00%> (ø)
...r/loadbalance/extensions/models/UnloadCounter.java 93.84% <ø> (ø)
...ulsar/broker/stats/prometheus/metrics/Summary.java 100.00% <ø> (ø)
...lance/extensions/channel/StateChangeListeners.java 84.61% <80.00%> (-3.62%) ⬇️
...va/org/apache/pulsar/broker/service/ServerCnx.java 71.98% <61.53%> (-0.07%) ⬇️
.../loadbalance/extensions/manager/UnloadManager.java 87.75% <89.28%> (+6.12%) ⬆️

... and 65 files with indirect coverage changes

Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM

@lhotari lhotari merged commit 176bdea into apache:master Jan 11, 2024
48 of 49 checks passed
@onobc
Copy link
Contributor Author

onobc commented Jan 17, 2024

@onobc Should we also add a mention in the Java client docs page on how to use the BOM?

Sorry, I missed this comment @merlimat . Yes, that is a good idea. I can add that.

@lhotari lhotari modified the milestones: 3.3.0, 3.2.0 Jan 21, 2024
lhotari pushed a commit that referenced this pull request Jan 21, 2024
Co-authored-by: Chris Bono <[email protected]>
(cherry picked from commit 176bdea)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants