[FLINK-12024] Bump universal Kafka connector to Kafka dependency to 2.2.0 #8055

yanghua · 2019-03-27T02:47:31Z

What is the purpose of the change

This pull request bumps universal Kafka connector to Kafka dependency to 2.2.0

Brief change log

Bump universal Kafka connector to Kafka dependency to 2.2.0

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): (yes / no)
The public API, i.e., is any changed class annotated with @Public(Evolving): (yes / no)
The serializers: (yes / no / don't know)
The runtime per-record code paths (performance sensitive): (yes / no / don't know)
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes / no / don't know)
The S3 file system connector: (yes / no / don't know)

Documentation

Does this pull request introduce a new feature? (yes / no)
If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

flinkbot · 2019-03-27T02:47:52Z

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Review Progress

❓ 1. The [description] looks good.
❓ 2. There is [consensus] that the contribution should go into to Flink.
❓ 3. Needs [attention] from.
❓ 4. The change fits into the overall [architecture].
❓ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.

Details

The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
@flinkbot approve all to approve all aspects
@flinkbot approve-until architecture to approve everything until architecture
@flinkbot attention @username1 [@username2 ..] to require somebody's attention
@flinkbot disapprove architecture to remove an approval you gave earlier

yanghua · 2019-03-27T05:48:06Z

cc @aljoscha and @pnowojski

zentol · 2019-03-27T15:01:31Z

No problems in terms of licensing.

bolkedebruin · 2019-03-27T16:16:00Z

Please, this fixes some really annoying bugs in Kafka

eliaslevy · 2019-03-27T16:51:00Z

While the universal tries to track the latest Kafka client version, at times it may fall behind. E.g. before this update it was using 2.0.1, skipping the 2.1.x series entirely. At the moment the version of the client used for the universal connector is not mentioned in the Flink documentation. It may be useful to do so. Users may be interested is knowing the specific version so they can tell whether specific features or fixes are included in the connector.

yanghua · 2019-03-28T09:45:26Z

@eliaslevy Your comment makes sense. I agree with you. I suggest we should describe the current kafka client version which the universal kafka connector tracks in the documentation. Any time when we upgrade the kafka's version, we also should update the document. What do you think? @pnowojski @aljoscha @zentol

aljoscha · 2019-03-28T09:54:52Z

I think this makes sense, yes.

yanghua · 2019-03-28T10:52:34Z

@aljoscha OK, thanks for your reply. I will remark the kafka client version in the document.

yanghua · 2019-03-28T11:13:38Z

docs/dev/connectors/kafka.md

        <td>FlinkKafkaConsumer<br>
        FlinkKafkaProducer</td>
-        <td>>= 1.0.0</td>
+        <td>>= 1.0.0 (until 2.2.0)</td>


@aljoscha and @pnowojski What do you think about this change?

eliaslevy · 2019-03-28T14:02:56Z

That's no good, because the Kafka client is also forward compatible, except for be features. An that is needed is just a comment stating the version used.

…

On Thu, Mar 28, 2019 at 6:55 AM vinoyang ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In docs/dev/connectors/kafka.md <#8055 (comment)>: > @@ -85,7 +85,7 @@ For most users, the `FlinkKafkaConsumer08` (part of `flink-connector-kafka`) is <td>1.7.0</td> <td>FlinkKafkaConsumer<br> FlinkKafkaProducer</td> - <td>>= 1.0.0</td> + <td>>= 1.0.0 (until 2.2.0)</td> @aljoscha <https://github.com/aljoscha> and @pnowojski <https://github.com/pnowojski> What do you think about this change? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8055 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAsQ4nHbn53NEzXjFyi3V0AYAVYFr3Keks5vbKP2gaJpZM4cM1us> .

yanghua · 2019-03-28T14:10:57Z

@eliaslevy Sounds reasonable. My change will make the users confused.

yanghua · 2019-03-28T14:17:26Z

docs/dev/connectors/kafka.md

        <td>
        This universal Kafka connector attempts to track the latest version of the Kafka client.
-        The version of the client it uses may change between Flink releases.
+        The version of the client it uses may change between Flink releases. Now, it uses Kafka client 2.2.0.


@eliaslevy What about this change?

eliaslevy · 2019-03-28T15:14:34Z

Good enough, but I would rephrase it as "As of this release, it uses the Kafka 2.2.0 client."

yanghua · 2019-03-28T16:02:23Z

@eliaslevy Thanks for your suggestion, accepted your opinion.

pnowojski

Thanks for the PR and the reviews :) One comment/question: what about other occurrences of the 2.0.1 string in the code base? Shouldn't they be changed as well?

git grep "2\.0\.1"
flink-connectors/flink-sql-connector-kafka/src/main/resources/META-INF/NOTICE:- org.apache.kafka:kafka-clients:2.0.1
flink-end-to-end-tests/flink-sql-client-test/pom.xml:                           <version>2.0.1</version>
flink-end-to-end-tests/test-scripts/test_sql_client.sh:KAFKA_VERSION="2.0.1"
flink-end-to-end-tests/test-scripts/test_sql_client_kafka.sh:source "$(dirname "$0")"/test_sql_client_kafka_common.sh 2.0 2.0.1 5.0.0 5.0 "kafka" "universal"
flink-end-to-end-tests/test-scripts/test_streaming_kafka.sh:source "$(dirname "$0")"/kafka-common.sh 2.0.1 5.0.0 5.0
flink-filesystems/flink-fs-hadoop-shaded/src/main/resources/META-INF/NOTICE:Azure Data Lake Store - Java client SDK 2.0.11
flink-jepsen/src/jepsen/flink/flink.clj:(def kafka-dist-url "http://mirror.funkfreundelandshut.de/apache/kafka/2.0.1/kafka_2.11-2.0.1.tgz")

yanghua · 2019-04-09T02:48:08Z

@pnowojski Thanks for pointing these version number. I have upgraded them to 2.2.0 except :
flink-filesystems/flink-fs-hadoop-shaded/src/main/resources/META-INF/NOTICE:Azure Data Lake Store - Java client SDK 2.0.11.

Moreover, I found the first arg KAFKA_CONNECTOR_VERSION in test_sql_client_kafka_common.sh seems useless. I suggest that we should remove them in another PR in the future.

pnowojski

In that case @yanghua it would be much better to drop this KAFKA_CONNECTOR_VERSION in this PR (as separate commit, before upgrading the connector version). Also can you resolve conflict in this PR?

pnowojski · 2019-04-17T07:27:14Z

flink-end-to-end-tests/test-scripts/test_sql_client.sh

@@ -20,7 +20,7 @@
 set -Eeuo pipefail

 KAFKA_CONNECTOR_VERSION="2.0"


Either please update it for a consistency or drop the field in this PR (as separate commit, before upgrading the connector version). As it is now, it's confusing and misleading.

OK, I will drop it in a separate commit.

@pnowojski I have finished this work, can you review again? thanks.

pnowojski · 2019-04-17T07:27:33Z

flink-end-to-end-tests/test-scripts/test_sql_client_kafka.sh

 set -Eeuo pipefail

-source "$(dirname "$0")"/test_sql_client_kafka_common.sh 2.0 2.0.1 5.0.0 5.0 "kafka" "universal"
+source "$(dirname "$0")"/test_sql_client_kafka_common.sh 2.0 2.2.0 5.0.0 5.0 "kafka" "universal"


ditto 2.0

….2.0

pnowojski

Thanks for the change @yanghua. LGTM and merging :)

….2.0 (apache#8055) This commit also removes useless arg:KAFKA_CONNECTOR_VERSION

rmetzger added review=description? component=Connectors/Kafka labels Mar 27, 2019

yanghua force-pushed the FLINK-12024 branch from 2e82204 to 6cf39cc Compare March 27, 2019 03:13

yanghua force-pushed the FLINK-12024 branch from 6cf39cc to 9d6c5a8 Compare March 28, 2019 11:12

yanghua commented Mar 28, 2019

View reviewed changes

yanghua force-pushed the FLINK-12024 branch from 9d6c5a8 to 7ccc87a Compare March 28, 2019 14:16

yanghua commented Mar 28, 2019

View reviewed changes

yanghua force-pushed the FLINK-12024 branch from 7ccc87a to 9ff1071 Compare March 28, 2019 15:43

pnowojski requested changes Apr 8, 2019

View reviewed changes

yanghua force-pushed the FLINK-12024 branch from 9ff1071 to 89dd179 Compare April 9, 2019 02:43

pnowojski requested changes Apr 17, 2019

View reviewed changes

yanghua force-pushed the FLINK-12024 branch from 89dd179 to 1240e68 Compare April 17, 2019 13:03

yanghua added 2 commits April 18, 2019 11:18

[FLINK-12024] Bump universal Kafka connector to Kafka dependency to 2…

93d5c4a

….2.0

Remove useless arg:KAFKA_CONNECTOR_VERSION

1cb21ad

yanghua force-pushed the FLINK-12024 branch from 1240e68 to 1cb21ad Compare April 18, 2019 03:19

yanghua mentioned this pull request Apr 23, 2019

[FLINK-10967] Update kafka dependency to 2.2.0 #7156

Closed

pnowojski approved these changes Apr 25, 2019

View reviewed changes

pnowojski merged commit 40ebe25 into apache:master Apr 25, 2019

tianchen92 pushed a commit to tianchen92/flink that referenced this pull request May 13, 2019

[FLINK-12024] Bump universal Kafka connector to Kafka dependency to 2…

25ec2ba

….2.0 (apache#8055) This commit also removes useless arg:KAFKA_CONNECTOR_VERSION

jnh5y pushed a commit to jnh5y/flink that referenced this pull request Dec 18, 2023

[FLINK-12024] Bump universal Kafka connector to Kafka dependency to 2…

263e62d

….2.0 (apache#8055) This commit also removes useless arg:KAFKA_CONNECTOR_VERSION

		@@ -20,7 +20,7 @@
		set -Eeuo pipefail

		KAFKA_CONNECTOR_VERSION="2.0"

[FLINK-12024] Bump universal Kafka connector to Kafka dependency to 2.2.0 #8055

[FLINK-12024] Bump universal Kafka connector to Kafka dependency to 2.2.0 #8055

Uh oh!

Conversation

yanghua commented Mar 27, 2019

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

Uh oh!

flinkbot commented Mar 27, 2019

Review Progress

Uh oh!

yanghua commented Mar 27, 2019

Uh oh!

zentol commented Mar 27, 2019

Uh oh!

bolkedebruin commented Mar 27, 2019

Uh oh!

eliaslevy commented Mar 27, 2019

Uh oh!

yanghua commented Mar 28, 2019

Uh oh!

aljoscha commented Mar 28, 2019

Uh oh!

yanghua commented Mar 28, 2019

Uh oh!

yanghua Mar 28, 2019

Choose a reason for hiding this comment

Uh oh!

eliaslevy commented Mar 28, 2019 via email

Uh oh!

yanghua commented Mar 28, 2019

Uh oh!

yanghua Mar 28, 2019

Choose a reason for hiding this comment

Uh oh!

eliaslevy commented Mar 28, 2019

Uh oh!

yanghua commented Mar 28, 2019

Uh oh!

pnowojski left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yanghua commented Apr 9, 2019

Uh oh!

pnowojski left a comment

Choose a reason for hiding this comment

Uh oh!

pnowojski Apr 17, 2019

Choose a reason for hiding this comment

Uh oh!

yanghua Apr 17, 2019

Choose a reason for hiding this comment

Uh oh!

yanghua Apr 18, 2019

Choose a reason for hiding this comment

Uh oh!

pnowojski Apr 17, 2019

Choose a reason for hiding this comment

Uh oh!

pnowojski left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

pnowojski left a comment •

edited

Loading