Skip to content

MINOR: avoid miss-leading log statement for KafkaStreams#close()#5963

Merged
mjsax merged 1 commit intoapache:2.1from
mjsax:improve-close-21
Nov 30, 2018
Merged

MINOR: avoid miss-leading log statement for KafkaStreams#close()#5963
mjsax merged 1 commit intoapache:2.1from
mjsax:improve-close-21

Conversation

@mjsax
Copy link
Copy Markdown
Member

@mjsax mjsax commented Nov 28, 2018

Calling KafkaStream#close() should not call deprecated #close() because this will log miss leading log statement about using a deprecated method.

@mjsax mjsax added the streams label Nov 28, 2018
@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Nov 28, 2018

Call for review @guozhangwang @bbejeck @vvcephei @ijuma

This fix is also contained in #5954 for trunk.

*/
public void close() {
close(DEFAULT_CLOSE_TIMEOUT, TimeUnit.SECONDS);
close(Long.MAX_VALUE);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We translate old default of zero to Long.MAX_VALUE within deprecated close(final long timeout, final TimeUnit timeUnit) -- we can call private close() directly instead.

Copy link
Copy Markdown
Member

@bbejeck bbejeck left a comment

Choose a reason for hiding this comment

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

@mjsax, thanks for the cleanup LGTM

@mjsax mjsax merged commit c142224 into apache:2.1 Nov 30, 2018
@mjsax mjsax deleted the improve-close-21 branch November 30, 2018 01:51
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.

3 participants