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

Pulsar with Bk 4.15.0 #15142

Merged
merged 5 commits into from
May 10, 2022
Merged

Pulsar with Bk 4.15.0 #15142

merged 5 commits into from
May 10, 2022

Conversation

dlg99
Copy link
Contributor

@dlg99 dlg99 commented Apr 12, 2022

Modifications

Upgraded BK to 4.15.0
Upgraded dependendencies, where applicable (license check to follow)
Fixed build breaks caused by BK, fixed tests etc (what's broken by new code or flakes I caught, was not sure if related)

Verifying this change

  • Make sure that the change passes the CI checks.

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

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): YES
  • The public API: (yes / no)
  • The schema: (yes / no / don't know)
  • The default values of configurations: PROBABLY (RocksDB)
  • The wire protocol: (yes / no)
  • The rest endpoints: (yes / no)
  • The admin cli options: (yes / no)
  • Anything that affects deployment: don't know

Documentation

Check the box below or label this PR directly.

Need to update docs?

Needs release notes, probably

  • doc-required
    (Your PR needs to update docs and you will update later)

  • no-need-doc
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-added
    (Docs have been already added)

@dlg99 dlg99 marked this pull request as draft April 12, 2022 22:27
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Apr 12, 2022
@dlg99 dlg99 force-pushed the bk-4.15 branch 14 times, most recently from 7b39a98 to 3a5387f Compare April 13, 2022 04:18
@eolivelli
Copy link
Contributor

You can add the repository for bk 4.15.0 rc0
Like I did here
https://github.com/diennea/herddb/pull/784/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R911

This way we can run CI against the RC

@merlimat merlimat added this to the 2.11.0 milestone Apr 13, 2022
@dlg99 dlg99 changed the title WIP: Pulsar with Bk 4.15 (4.16-snapshot currently, as a placeholder for rc1 of 4.15) WIP: Pulsar with Bk 4.15.0 (RC1 currently) Apr 27, 2022
* RxJava
- io.reactivex.rxjava3-rxjava-3.0.1.jar
* RabbitMQ Java Client
- com.rabbitmq-amqp-client-5.5.3.jar
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need these 2 new libraries?

Copy link
Contributor Author

@dlg99 dlg99 May 5, 2022

Choose a reason for hiding this comment

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

mvn dependency:tree:

[INFO] +- org.apache.bookkeeper:bookkeeper-server:jar:4.15.0:compile
...
[INFO] |  +- io.reactivex.rxjava3:rxjava:jar:3.0.1:compile
[INFO] |  |  \- org.reactivestreams:reactive-streams:jar:1.0.3:compile


[INFO] +- org.apache.bookkeeper.stats:codahale-metrics-provider:jar:4.15.0:compile
[INFO] |  +- io.dropwizard.metrics:metrics-jmx:jar:4.1.12.1:compile
[INFO] |  +- io.dropwizard.metrics:metrics-jvm:jar:4.1.12.1:compile
[INFO] |  \- io.dropwizard.metrics:metrics-graphite:jar:4.1.12.1:compile
[INFO] |     \- com.rabbitmq:amqp-client:jar:5.5.3:compile

RxJava as result of apache/bookkeeper#2936
amqp-client as part of dropwizard upgrade

Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're not going to use RabbitMQ client to export metrics, we could just exclude these dependencies.

Copy link
Contributor

Choose a reason for hiding this comment

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

More over, I'm not sure why we're importing codahale-metrics-provider from managed-ledger. I think it's not needed at all :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Anyway, this should be handled separately and we should merge this now.

@dlg99 dlg99 changed the title WIP: Pulsar with Bk 4.15.0 (RC1 currently) WIP: Pulsar with Bk 4.15.0 (RC2 currently) May 5, 2022
@dlg99 dlg99 changed the title WIP: Pulsar with Bk 4.15.0 (RC2 currently) Pulsar with Bk 4.15.0 May 9, 2022
@dlg99 dlg99 marked this pull request as ready for review May 9, 2022 22:02
@merlimat merlimat merged commit 7a7b5fa into apache:master May 10, 2022
}
return registrationManager;
}

Copy link
Member

Choose a reason for hiding this comment

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

Why do we make this change? It seems we generate an unreleased resource here.

cc @dlg99

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants