[FLINK-11015] Remove deprecated methods and classes about table from all the Kafka connectors #7182

yanghua · 2018-11-27T16:41:59Z

What is the purpose of the change

This pull request removes deprecated methods and classes about table from all the Kafka connectors

Brief change log

Remove deprecated methods and classes about table from all the Kafka connectors

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)

yanghua · 2018-11-28T07:22:50Z

@twalthr Can you review this PR? It blocks FLINK-9461.

yanghua · 2018-11-28T08:36:48Z

cc @pnowojski

twalthr

Thank you @yanghua. The removal of table-related classes looks good to me. However, the work is not complete we also need to:

solve TODOs in org.apache.flink.streaming.connectors.kafka.KafkaTableSinkBase and org.apache.flink.streaming.connectors.kafka.KafkaTableSourceBase
remove all deprecated methods in org.apache.flink.streaming.connectors.kafka.KafkaTableSinkBase and org.apache.flink.streaming.connectors.kafka.KafkaTableSourceBase
remove everything commented with legacy in org.apache.flink.streaming.connectors.kafka.KafkaTableSinkBase
remove builder in org.apache.flink.streaming.connectors.kafka.KafkaTableSourceBase

I'm also wondering why you remove classes that have nothing to do with flink-table. Such as TransactionalIdsGeneratorTest can you explain that.

twalthr · 2018-11-28T08:33:03Z

...1/src/test/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducer011ITCase.java

- * IT cases for the {@link FlinkKafkaProducer011}.
- */
-@SuppressWarnings("serial")
-public class FlinkKafkaProducer011ITCase extends KafkaTestBase {


This change has nothing to do with a flink-table free Kafka module. It should be moved to a separate PR or at least separate commit. Why can we remove this class now?

Sorry This is an operational mistake, I should be more careful.

yanghua · 2018-11-28T09:43:33Z

@twalthr thanks for your suggestion. I have checked the list you gave. I found the builder in org.apache.flink.streaming.connectors.kafka.KafkaTableSourceBase has been deleted before you point.

…all the Kafka connectors

twalthr

Thanks for the update @yanghua. One additional comment. We should also update the dependencies of the Maven modules.

twalthr · 2018-11-28T10:17:18Z

...kafka-base/src/main/java/org/apache/flink/streaming/connectors/kafka/KafkaTableSinkBase.java


 	/** Partitioner to select Kafka partition for each item. */
 	protected final Optional<FlinkKafkaPartitioner<Row>> partitioner;

-	// legacy variables
 	protected String[] fieldNames;


I think we also don't need these variables anymore.

twalthr · 2018-11-28T13:48:34Z

flink-connectors/flink-connector-kafka-0.10/pom.xml

-			<groupId>org.apache.flink</groupId>
-			<artifactId>flink-json</artifactId>
-			<version>${project.version}</version>
-			<type>test-jar</type>


@yanghua Do we need to generate test jars for flink-json and flink-avro anymore?

@twalthr The specific test cases have been removed and there is no error when building by maven. Did I miss anything else?

In order to share test classes across modules a module must build test-jars. If the test jar of flink-json and flink-avro are not necessary anymore, we can remove the jar building section from those pom.xml.

My own Travis has turned green (Can not make sure if you can see this link). OK, It would be better to wait and see Travis's result to verify the judgment.

@yanghua I'm waiting for the OK from @zentol. Dependency changes should always be reviewed by a couple of people.

yanghua · 2018-11-29T07:16:37Z

@twalthr Any other changes? If not, I look forward to merging it as soon as possible to advance FLINK-9461. Thanks.

zentol · 2018-11-29T09:23:04Z

+1 to the pom changes.

twalthr · 2018-11-29T09:29:45Z

Thank you @zentol. I will take care of merging this...

yanghua · 2018-11-30T06:40:05Z

@twalthr Let's merge this PR? thanks.

twalthr · 2018-11-30T11:18:36Z

flink-connectors/flink-connector-kafka-base/pom.xml

@@ -44,12 +44,6 @@ under the License.

 		<!-- core dependencies -->

-		<dependency>


We need to reintroduce this dependency as org.apache.flink.streaming.util.serialization.JSONKeyValueDeserializationSchema still requires Jackson. I discussed this with Chesnay. I will reintroduce it while merging.

…connectors This commit removes all classes and methods that have been deprecated in Flink 1.6 for separating Kafka connectors from Avro and JSON formats. This closes apache#7182.

yanghua force-pushed the FLINK-11015 branch from 86d2288 to 7ac8710 Compare November 28, 2018 01:07

twalthr requested changes Nov 28, 2018

View reviewed changes

yanghua force-pushed the FLINK-11015 branch from 7ac8710 to 0ffa1e5 Compare November 28, 2018 09:29

[FLINK-11015] Remove deprecated methods and classes about table from …

6815e64

…all the Kafka connectors

yanghua force-pushed the FLINK-11015 branch from 0ffa1e5 to 6815e64 Compare November 28, 2018 09:54

twalthr requested changes Nov 28, 2018

View reviewed changes

refactor based on review suggestion

8224bcc

twalthr reviewed Nov 28, 2018

View reviewed changes

twalthr self-assigned this Nov 28, 2018

twalthr reviewed Nov 30, 2018

View reviewed changes

asfgit closed this in 7d2c269 Nov 30, 2018

rmetzger added the component=Connectors/Kafka label Mar 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLINK-11015] Remove deprecated methods and classes about table from all the Kafka connectors #7182

[FLINK-11015] Remove deprecated methods and classes about table from all the Kafka connectors #7182

yanghua commented Nov 27, 2018

yanghua commented Nov 28, 2018

yanghua commented Nov 28, 2018

twalthr left a comment •

edited

twalthr Nov 28, 2018

yanghua Nov 28, 2018

yanghua commented Nov 28, 2018

twalthr left a comment

twalthr Nov 28, 2018

yanghua Nov 28, 2018

twalthr Nov 28, 2018

yanghua Nov 28, 2018

twalthr Nov 28, 2018

yanghua Nov 28, 2018 •

edited

twalthr Nov 29, 2018

yanghua commented Nov 29, 2018

zentol commented Nov 29, 2018

twalthr commented Nov 29, 2018

yanghua commented Nov 30, 2018

twalthr Nov 30, 2018

yanghua Nov 30, 2018

		@@ -44,12 +44,6 @@ under the License.

		<!-- core dependencies -->

		<dependency>

[FLINK-11015] Remove deprecated methods and classes about table from all the Kafka connectors #7182

[FLINK-11015] Remove deprecated methods and classes about table from all the Kafka connectors #7182

Conversation

yanghua commented Nov 27, 2018

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

yanghua commented Nov 28, 2018

yanghua commented Nov 28, 2018

twalthr left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yanghua commented Nov 28, 2018

twalthr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yanghua Nov 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yanghua commented Nov 29, 2018

zentol commented Nov 29, 2018

twalthr commented Nov 29, 2018

yanghua commented Nov 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

twalthr left a comment •

edited

yanghua Nov 28, 2018 •

edited