Skip to content

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented Jul 30, 2016

What changes were proposed in this pull request?

This PR replaces the old Kafka API to 0.10.0 ones in KafkaTestUtils.

The change include:

  • Producer to KafkaProducer
  • Change configurations to equalvant ones. (I referred here for 0.10.0 and here for old, 0.8.2).

This PR will remove the build warning as below:

[WARNING] .../spark/external/kafka-0-10/src/main/scala/org/apache/spark/streaming/kafka010/KafkaTestUtils.scala:71: class Producer in package producer is deprecated: This class has been deprecated and will be removed in a future release. Please use org.apache.kafka.clients.producer.KafkaProducer instead.
[WARNING]   private var producer: Producer[String, String] = _
[WARNING]                         ^
[WARNING] .../spark/external/kafka-0-10/src/main/scala/org/apache/spark/streaming/kafka010/KafkaTestUtils.scala:181: class Producer in package producer is deprecated: This class has been deprecated and will be removed in a future release. Please use org.apache.kafka.clients.producer.KafkaProducer instead.
[WARNING]     producer = new Producer[String, String](new ProducerConfig(producerConfiguration))
[WARNING]                    ^
[WARNING] .../spark/streaming/kafka010/KafkaTestUtils.scala:181: class ProducerConfig in package producer is deprecated: This class has been deprecated and will be removed in a future release. Please use org.apache.kafka.clients.producer.ProducerConfig instead.
[WARNING]     producer = new Producer[String, String](new ProducerConfig(producerConfiguration))
[WARNING]                                                 ^
[WARNING] .../spark/external/kafka-0-10/src/main/scala/org/apache/spark/streaming/kafka010/KafkaTestUtils.scala:182: class KeyedMessage in package producer is deprecated: This class has been deprecated and will be removed in a future release. Please use org.apache.kafka.clients.producer.ProducerRecord instead.
[WARNING]     producer.send(messages.map { new KeyedMessage[String, String](topic, _ ) }: _*)
[WARNING]                                      ^
[WARNING] four warnings found
[WARNING] warning: [options] bootstrap class path not set in conjunction with -source 1.7
[WARNING] 1 warning

How was this patch tested?

Existing tests that use KafkaTestUtils should cover this.

@HyukjinKwon
Copy link
Member Author

Could you please take a look, @koeninger and @holdenk ?

@HyukjinKwon
Copy link
Member Author

Please let me cc @srowen as well because it is anyway about building.

producer.send(messages.map { new KeyedMessage[String, String](topic, _ ) }: _*)
producer = new KafkaProducer[String, String](producerConfiguration)
val records = messages.map { new ProducerRecord[String, String](topic, _) }
records.map(producer.send)
Copy link
Contributor

Choose a reason for hiding this comment

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

super minor: could use foreach instead of map since we don't care about the return value.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, thank you.

Copy link
Member

Choose a reason for hiding this comment

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

This type of thing is even essential for correctness in some cases, like https://issues.apache.org/jira/browse/SPARK-16664

@holdenk
Copy link
Contributor

holdenk commented Jul 30, 2016

This looks reasonable pending tests and verifying no more kafka deprecation warnings in the build logs for this file :)

@SparkQA
Copy link

SparkQA commented Jul 30, 2016

Test build #63033 has finished for PR 14416 at commit bb9f635.

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

@SparkQA
Copy link

SparkQA commented Jul 30, 2016

Test build #63038 has finished for PR 14416 at commit b7a53f2.

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

producer.send(messages.map { new KeyedMessage[String, String](topic, _ ) }: _*)
producer = new KafkaProducer[String, String](producerConfiguration)
val records = messages.map { new ProducerRecord[String, String](topic, _) }
records.foreach(producer.send)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the point here was that you don't need to map at all; messages.foreach should suffice.

@koeninger
Copy link
Contributor

LGTM, minor comments about map notwithstanding.

@SparkQA
Copy link

SparkQA commented Jul 30, 2016

Test build #63044 has finished for PR 14416 at commit a1ef311.

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

@srowen
Copy link
Member

srowen commented Aug 1, 2016

Merged to master/2.0

@asfgit asfgit closed this in f93ad4f Aug 1, 2016
asfgit pushed a commit that referenced this pull request Aug 1, 2016
… 0.10.0.

## What changes were proposed in this pull request?

This PR replaces the old Kafka API to 0.10.0 ones in `KafkaTestUtils`.

The change include:

 - `Producer` to `KafkaProducer`
 - Change configurations to equalvant ones. (I referred [here](http://kafka.apache.org/documentation.html#producerconfigs) for 0.10.0 and [here](http://kafka.apache.org/082/documentation.html#producerconfigs
) for old, 0.8.2).

This PR will remove the build warning as below:

```scala
[WARNING] .../spark/external/kafka-0-10/src/main/scala/org/apache/spark/streaming/kafka010/KafkaTestUtils.scala:71: class Producer in package producer is deprecated: This class has been deprecated and will be removed in a future release. Please use org.apache.kafka.clients.producer.KafkaProducer instead.
[WARNING]   private var producer: Producer[String, String] = _
[WARNING]                         ^
[WARNING] .../spark/external/kafka-0-10/src/main/scala/org/apache/spark/streaming/kafka010/KafkaTestUtils.scala:181: class Producer in package producer is deprecated: This class has been deprecated and will be removed in a future release. Please use org.apache.kafka.clients.producer.KafkaProducer instead.
[WARNING]     producer = new Producer[String, String](new ProducerConfig(producerConfiguration))
[WARNING]                    ^
[WARNING] .../spark/streaming/kafka010/KafkaTestUtils.scala:181: class ProducerConfig in package producer is deprecated: This class has been deprecated and will be removed in a future release. Please use org.apache.kafka.clients.producer.ProducerConfig instead.
[WARNING]     producer = new Producer[String, String](new ProducerConfig(producerConfiguration))
[WARNING]                                                 ^
[WARNING] .../spark/external/kafka-0-10/src/main/scala/org/apache/spark/streaming/kafka010/KafkaTestUtils.scala:182: class KeyedMessage in package producer is deprecated: This class has been deprecated and will be removed in a future release. Please use org.apache.kafka.clients.producer.ProducerRecord instead.
[WARNING]     producer.send(messages.map { new KeyedMessage[String, String](topic, _ ) }: _*)
[WARNING]                                      ^
[WARNING] four warnings found
[WARNING] warning: [options] bootstrap class path not set in conjunction with -source 1.7
[WARNING] 1 warning
```

## How was this patch tested?

Existing tests that use `KafkaTestUtils` should cover this.

Author: hyukjinkwon <[email protected]>

Closes #14416 from HyukjinKwon/SPARK-16776.

(cherry picked from commit f93ad4f)
Signed-off-by: Sean Owen <[email protected]>
@HyukjinKwon HyukjinKwon deleted the SPARK-16776 branch January 2, 2018 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants