Skip to content

KAFKA-12864: Move KafkaEventQueue into server-common#10787

Merged
cmccabe merged 1 commit into
apache:trunkfrom
cmccabe:KAFKA-12864
Jun 2, 2021
Merged

KAFKA-12864: Move KafkaEventQueue into server-common#10787
cmccabe merged 1 commit into
apache:trunkfrom
cmccabe:KAFKA-12864

Conversation

@cmccabe

@cmccabe cmccabe commented May 28, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Move the KafkaEventQueue into server-common. Since it is a generic data structure not specific to metadata, it fits better into the server-common gradle module than into the metadata module.

@cmccabe cmccabe added the kraft label May 28, 2021
@ijuma

ijuma commented May 30, 2021

Copy link
Copy Markdown
Member

Thanks for the PR. Can we please include the motivation?

@ijuma

ijuma commented Jun 2, 2021

Copy link
Copy Markdown
Member

Are we using these classes from any module besides metadata atm?

mumrah
mumrah approved these changes Jun 2, 2021
View reviewed changes

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

LGTM pending tests, thanks @cmccabe 👍

@ijuma

ijuma commented Jun 2, 2021

Copy link
Copy Markdown
Member

I would personally not move these classes unless we believe we will use them from other modules. They're a bit specialized, especially the timeline ones.

@cmccabe

cmccabe commented Jun 2, 2021

Copy link
Copy Markdown
Contributor Author

I would personally not move these classes unless we believe we will use them from other modules. They're a bit specialized, especially the timeline ones.

KafkaEventQueue is already used in core. It's just an event queue; there is nothing metadata-specific about it, so I think it belongs in server-common. The timeline data structures are not used anywhere else, however.

I will revise the PR to move just the queue and not the timeline data structures.

@cmccabe cmccabe changed the title KAFKA-12864: Move queue and timeline into server-common KAFKA-12864: Move KafkaEventQueue into server-common Jun 2, 2021
@ijuma

ijuma commented Jun 2, 2021

Copy link
Copy Markdown
Member

Makes sense. Thanks!

@cmccabe

cmccabe commented Jun 2, 2021

Copy link
Copy Markdown
Contributor Author

OK. Thanks for the reviews. I will commit after Jenkins runs if there are no objections

(current PR just moves KafkaEventQueue)

ijuma
ijuma approved these changes Jun 2, 2021
View reviewed changes

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

LGTM. Let's update the PR description and commit message too.

@cmccabe cmccabe merged commit ccec9b0 into apache:trunk Jun 2, 2021
@cmccabe cmccabe deleted the KAFKA-12864 branch June 2, 2021 23:11
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

@mumrah mumrah mumrah approved these changes

Assignees

No one assigned

Labels

kraft

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cmccabe @ijuma @mumrah