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

KAFKA-6659: Improve error message if state store is not found #4732

Merged
merged 4 commits into from
Mar 22, 2018

Conversation

perkss
Copy link
Contributor

@perkss perkss commented Mar 19, 2018

Update the exception message thrown to state about the state store not found and suggest checking store has been connected to the processor.

@perkss
Copy link
Contributor Author

perkss commented Mar 19, 2018

@mjsax please take a look. Thanks

@mjsax
Copy link
Member

mjsax commented Mar 19, 2018

\cc @bbejeck @vvcephei

@mjsax
Copy link
Member

mjsax commented Mar 19, 2018

\cc @astubbs

@astubbs
Copy link
Contributor

astubbs commented Mar 19, 2018

The problem with this new message is that most DSL users won’t know what it means. Would be food to somehow cover both bases?

@perkss
Copy link
Contributor Author

perkss commented Mar 20, 2018

@astubbs I could change the text from explicit you to just "Please check the store has been connected to the processor." I think this would make it more generic or we use the terminology about adding state store?

@mjsax
Copy link
Member

mjsax commented Mar 20, 2018

Maybe we can be more explicit. Something like this:

"Processor " + currentNode().name() + " has no access to StateStore " + name + " as the store is not connected to the processor."
+ " If you add stores manually via '.addStateStore()' make sure to connect the added store to the processor by providing the processor name to '.addStateStore()' or connect them via '.connectProcessorAndStateStores()'."
+ " DSL users need to provide the store name to '.process()', '.transform()', or '.transformValues()' to connect the store to the corresponding operator."
+ " If you don't add stores manually, please file a bug report at https://issues.apache.org/jira/projects/KAFKA."

@perkss
Copy link
Contributor Author

perkss commented Mar 20, 2018

@astubbs what do you think of @mjsax suggestion?

Copy link
Member

@mjsax mjsax left a comment

Choose a reason for hiding this comment

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

LGTM.

Call for second review: @astubbs @guozhangwang @bbejeck @vvcephei

Copy link
Contributor

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

Thanks for the patch, LGTM

@mjsax
Copy link
Member

mjsax commented Mar 22, 2018

If there are no further comments, I'll merge this soon.

@mjsax mjsax merged commit 9ee00c4 into apache:trunk Mar 22, 2018
@mjsax
Copy link
Member

mjsax commented Mar 22, 2018

Merged to trunk.

Thanks for the patch @perkss !

@perkss perkss deleted the state-store-not-connected-error-message branch March 23, 2018 01:41
@perkss
Copy link
Contributor Author

perkss commented Mar 23, 2018

@mjsax @bbejeck thank you!

hejiefang pushed a commit to hejiefang/kafka that referenced this pull request Mar 26, 2018
…#4732)

Reviewers: Bill Bejeck <bill@confluent.io>, Matthias J. Sax <matthias@confluent.io>
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