kafka icon indicating copy to clipboard operation
kafka copied to clipboard

Do not allow consumer with empty list of subscriptions.

Open aloiscochard opened this issue 8 years ago • 3 comments

Hi, first thanks a lot for this great client!

We had a bug today, were we did pass an empty list (by mistake) of subscription to the strategy of GroupConsumer, it took us actually quite a while to workout what was the original issue as everything was getting initialized without error.

How do you feel about making GroupConsumer strategy initialization more strict, and throwing an error (or warn at least?) if a consumer is created with an empty list of subscriptions?

Thanks in advance

PS: We actually got a warning "No partition assignment received", but it did not help much investigating the core issue.

aloiscochard avatar Aug 03 '16 15:08 aloiscochard

I think the no partition assignment should be enough of an indication as to what is going on, especially now that it's an issue on github. I personally am not a big fan of enforcing constraints that aren't necessary. In our case we use the fact that we can send in an empty array to essentially create a mock object for integration tests. Just my 5ct on the matter :-)

RXminuS avatar Aug 17 '16 12:08 RXminuS

I think that expecting people to resolve a problem purely based on the fact that they will be able to find some keywords in a specific github issue, is non-sense.

When I read "No partition assignment received." I do not understand "No subscriptions defined", but maybe I'm just being stupid.

I can understand the mocking needs (even though other approach would work without requiring loose constraints), but really, I don't see any reason to not improve the warning message at least (I'm fine making the PR if people are okay with it).

aloiscochard avatar Aug 17 '16 13:08 aloiscochard

Oh yeah, for sure make the error message more descriptive!

But you were talking about making validation more strict...i.e. not possible to send in an empty list, v.s. still possible but just better message if you do.

RXminuS avatar Aug 17 '16 13:08 RXminuS