Skip to content

[improve][build] Remove broker-shaded artifacts - #3560

Merged
codelipenghui merged 2 commits into
apache:masterfrom
merlimat:remove-broker-shaded
Nov 16, 2022
Merged

[improve][build] Remove broker-shaded artifacts#3560
codelipenghui merged 2 commits into
apache:masterfrom
merlimat:remove-broker-shaded

Conversation

@merlimat

@merlimat merlimat commented Feb 9, 2019 •
edited by tisonkun
Loading

Copy link
Copy Markdown
Contributor

Motivation

The pulsar-broker-shaded is creating a huge jar with all broker deps. This was meant as a way to avoid dependencies conflicts when embedding Pulsar broker into an existing application. As far as I know, this is not currently used and it takes significant time and resources to assemble during the build.

I propose to remove for now, we can always bring it back when needed.

(cc/ @eolivelli just in case you happen to use this)

  • doc-not-needed

@merlimat merlimat added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Feb 9, 2019
@merlimat merlimat added this to the 2.3.0 milestone Feb 9, 2019
@merlimat merlimat self-assigned this Feb 9, 2019
eolivelli
eolivelli approved these changes Feb 9, 2019
View reviewed changes

@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.

I am not using it.
Thank you for considering my case @merlimat

rdhabalia
rdhabalia approved these changes Feb 9, 2019
View reviewed changes

@rdhabalia rdhabalia 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.

👍
should we send email/slack to pulsar-dev/user channel before removing it?

@sijie

sijie commented Feb 11, 2019

Copy link
Copy Markdown
Member

+1 to @rdhabalia 's comment. I think broker-shaded was introduced for addressing some requirements from slack users. it is not like the other shaded modules I removed in #3559

@merlimat

merlimat commented Feb 11, 2019

Copy link
Copy Markdown
Contributor Author

I know where it was coming from, though I think it was not used in the end. I know this is different from the 2 other shaded modules, but still a big time/resource sink to build each time, if there's no need for ti. Let's move this for 2.4 to have more time for gathering feedback.

@merlimat merlimat modified the milestones: 2.3.0, 2.4.0 Feb 11, 2019
@sijie sijie modified the milestones: 2.4.0, 2.5.0 Jun 9, 2019
@aahmed-se

aahmed-se commented Aug 26, 2019

Copy link
Copy Markdown
Contributor

@merlimat can you rebase this

eolivelli
eolivelli approved these changes Oct 26, 2019
View reviewed changes

@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.

Fine to me.
Even if I am using this artifact it is better to drop it, there is no much value.
I can check only on Monday at work, but IIRC we stopped using it

cc @diegosalvi

@eolivelli

eolivelli commented Oct 26, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

I suggest not to port this to 2.4 line but to ship it only with a major version

@sijie sijie modified the milestones: 2.5.0, 2.6.0 Nov 25, 2019
@codelipenghui

codelipenghui commented May 19, 2020

Copy link
Copy Markdown
Contributor

@merlimat Is this PR need to onboard the 2.6.0 release? or we can move it to 2.7.0?

@codelipenghui

codelipenghui commented May 25, 2020

Copy link
Copy Markdown
Contributor

move to 2.7.0

@codelipenghui codelipenghui modified the milestones: 2.6.0, 2.7.0 May 25, 2020
@codelipenghui codelipenghui modified the milestones: 2.7.0, 2.8.0 Nov 4, 2020
@codelipenghui codelipenghui removed this from the 2.8.0 milestone May 21, 2021
@codelipenghui codelipenghui added this to the 2.9.0 milestone May 21, 2021
@eolivelli

eolivelli commented Sep 23, 2021

Copy link
Copy Markdown
Contributor

can you please rebase ?
this patch was approved, it is good to commit it now for 2.9

@eolivelli eolivelli modified the milestones: 2.9.0, 2.10.0 Oct 6, 2021
@codelipenghui codelipenghui modified the milestones: 2.10.0, 2.11.0 Jan 18, 2022
@github-actions

github-actions Bot commented Mar 4, 2022

Copy link
Copy Markdown

@merlimat:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@codelipenghui codelipenghui modified the milestones: 2.11.0, 2.12.0 Jul 26, 2022
tisonkun
tisonkun approved these changes Nov 15, 2022
View reviewed changes

@tisonkun tisonkun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Rebased. If it's good to go, we can keep an eye on this PR :)

@tisonkun tisonkun changed the title Remove broker-shaded artifacts [improve][build] Remove broker-shaded artifacts Nov 15, 2022
@github-actions github-actions Bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Nov 15, 2022
@tisonkun

tisonkun commented Nov 15, 2022

Copy link
Copy Markdown
Member

Pending for merging...

cc @eolivelli @merlimat @rdhabalia @codelipenghui for a double check since it's stale for a while :)

@codelipenghui
codelipenghui merged commit cc63c6d into apache:master Nov 16, 2022
@merlimat

merlimat commented Nov 16, 2022

Copy link
Copy Markdown
Contributor Author

🎉🎉🎉 Thanks @tisonkun for merging this 3 years old PR! :)

@merlimat
merlimat deleted the remove-broker-shaded branch November 16, 2022 00:37
nodece pushed a commit to nodece/pulsar that referenced this pull request Jun 4, 2025
@merlimat @nodece
[improve][build] Remove broker-shaded artifacts (apache#3560)
99d3b90 
(cherry picked from commit cc63c6d)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ivankelly ivankelly ivankelly approved these changes
@rdhabalia rdhabalia rdhabalia approved these changes
@aahmed-se aahmed-se aahmed-se approved these changes
@eolivelli eolivelli eolivelli approved these changes
@tisonkun tisonkun tisonkun approved these changes
@michaeljmarshall michaeljmarshall michaeljmarshall approved these changes
@sijie sijie Awaiting requested review from sijie
@nkurihar nkurihar Awaiting requested review from nkurihar

Assignees

@merlimat merlimat

Labels

doc-not-needed Your PR changes do not impact docs lifecycle/stale type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages

Projects

None yet

Milestone

3.0.0

Development

Successfully merging this pull request may close these issues.

9 participants

@merlimat @sijie @aahmed-se @eolivelli @codelipenghui @tisonkun @ivankelly @rdhabalia @michaeljmarshall