flink icon indicating copy to clipboard operation
flink copied to clipboard

[FLINK-14068][core] Removes deprecated org.apache.flink.api.common.time.Time

Open XComp opened this issue 1 year ago • 3 comments

What is the purpose of the change

Removes deprecated org.apache.flink.api.common.time.Time

Brief change log

The changes where done using the IDE refactoring and find&replace operations.

Verifying this change

  • CI should pass

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

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

Documentation

  • Does this pull request introduce a new feature? no
  • If yes, how is the feature documented? not applicable

XComp avatar Aug 23 '24 21:08 XComp

CI report:

  • ab2e10649697c8e4c7b778274f6c1c8ad2a27d31 Azure: SUCCESS
Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

flinkbot avatar Aug 23 '24 22:08 flinkbot

Converting this one to draft. Things to do: ✅ Going through the code diff and identifying wrong/missing code changes ✅ Going through the docs diff and identifying wrong/missing changes ✅ Fixing Python API

XComp avatar Aug 29 '24 05:08 XComp

@flinkbot run azure

XComp avatar Sep 05 '24 21:09 XComp

@HuangXingBo @dianfu I am removing the Java class Time in this refactoring but struggle with the flink-python code. CI fails repeatedly with the following error:

Sep 05 18:51:05 >           raise Py4JError("{0} does not exist in the JVM".format(new_fqn))
Sep 05 18:51:05 E           py4j.protocol.Py4JError: org.apache.flink.streaming.connectors.kafka.FlinkKafkaProducer does not exist in the JVM

The error doesn't occur without my changes, so I assume that it's caused by the PR. But the error message seems unrelated (I didn't touch FlinkKafkaProducer). And I don't find any hint where to look for the cause of the issue.

Could you help a bit here or point me to someone who could?

XComp avatar Sep 06 '24 05:09 XComp

@flinkbot run azure

XComp avatar Sep 08 '24 11:09 XComp

@flinkbot run azure

XComp avatar Sep 08 '24 12:09 XComp

The error doesn't occur without my changes, so I assume that it's caused by the PR. But the error message seems unrelated (I didn't touch FlinkKafkaProducer). And I don't find any hint where to look for the cause of the issue.

Ok, looks like the Time class cannot be deleted because other libraries (e.g. flink-connector-kafka with [FlinkKafkaProducer:29](https://github.com/apache/flink-connector-kafka/blob/main/flink-connector-kafka/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducer.java#L29] still rely on it)

I overlooked the hint in the e2e test failure.

I reverted the removal of the Time class and labeled it as @Internal. Let's see whether that fixes the issue. :crossed_fingers:

XComp avatar Sep 09 '24 05:09 XComp

@zentol

XComp avatar Sep 13 '24 19:09 XComp

Thanks for picking this one up, @zentol . I addressed you one change request and will squash-merge the PR after another CI run.

The current CI failure is related to FLINK-36279.

XComp avatar Sep 16 '24 12:09 XComp

force-pushed a rebase to master after FLINK-36279 was merged.

XComp avatar Sep 16 '24 15:09 XComp