[SPARK-8865][STREAMING] FIX BUG: check key in kafka params

[guowei2]

No description provided.

[sarutak]

Could you add a test case to KafkaClusterSuite? Otherwise, LGTM.

[sarutak]

Also, could you add a brief description?

[SparkQA]

Test build #36648 has finished for PR 7254 at commit 48ca17a.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[srowen]

LGTM. The explanation is that contains == containsValue

[tdas]

@koeninger Please take a look at this. I am curious as to how it fails.

[koeninger]

"zookeeper.connect" and "group.id" aren't necessary for anything in the kafka direct stream.

But they're expected to be present in a kafka consumer config, and overriding that behavior wasn't possible. So as a workaround, we set them to a blank string. That way users don't have to define unnecessary settings in the kafka param map passed to the KafkaUtils constructor. We talked through that during the original development of the direct stream.

The code as it is released today is almost always going to set a blank string, regardless of what users pass in, because contains on a java property object is not the equivalent of containsKey, it is containsValue. This just fixes it so that if they do pass in values for those keys, for whatever personal reasons they have, the values won't get overwritten with a blank string.

TL;DR this should have no impact, but is a good bugfix so that the code does what it looks like it's supposed to.

[koeninger]

In case it wasn't clear, LGTM

I don't think it needs a test case, because it would be testing behavior that spark doesn't care about (we don't deal with group id or zookeeper connect)

[tdas]

@koeninger Thank you very much for the explanation. I am going to add this to the JIRa.
@guowei2 Thank you for catching this bug, I am going to merge this to master, branch 1.4 and branch-1.3