Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[FLINK-14068][streaming] Removes deprecated org.apache.flink.streaming.api.windowing.time.Time #25261

Merged
merged 2 commits into from
Sep 29, 2024
Merged

[FLINK-14068][streaming] Removes deprecated org.apache.flink.streaming.api.windowing.time.Time #25261

merged 2 commits into from
Sep 29, 2024

Conversation

XComp
Copy link
Contributor

@XComp XComp commented Aug 28, 2024

What is the purpose of the change

Removes deprecated org.apache.flink.streaming.api.windowing.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

@flinkbot
Copy link
Collaborator

flinkbot commented Aug 28, 2024 •
edited
Loading

CI report:

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

@XComp XComp marked this pull request as draft August 29, 2024 05:55
@XComp
Copy link
Contributor Author

XComp commented Aug 29, 2024 •
edited
Loading

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

@XComp XComp marked this pull request as ready for review August 29, 2024 17:01
@XComp XComp force-pushed the FLINK-14068-streaming branch 2 times, most recently from 86f1e06 to 1ed4049 Compare September 4, 2024 16:42
zentol
zentol reviewed Sep 10, 2024
View reviewed changes
docs/content.zh/docs/ops/state/task_failure_recovery.md
@@ -172,10 +172,10 @@ restart-strategy.exponential-delay.attempts-before-reset-backoff: 10
```java
StreamExecutionEnvironment env = StreamExecutionEnvironment.getExecutionEnvironment();
env.setRestartStrategy(RestartStrategies.exponentialDelayRestart(
Time.milliseconds(1),
Time.milliseconds(1000),
Duration.ofMillis(1),
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 this is wrong; this here uses the common Time class.

Copy link
Contributor

Choose a reason for hiding this comment

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

This wasn't addressed yet?

zentol
zentol reviewed Sep 10, 2024
View reviewed changes
Copy link
Contributor

@zentol zentol left a comment

Choose a reason for hiding this comment

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

Nice, looks mostly fine, but had a few comments.

@zentol zentol self-assigned this Sep 10, 2024
@XComp XComp force-pushed the FLINK-14068-streaming branch 2 times, most recently from e2d793b to 7711a3e Compare September 23, 2024 12:47
@XComp
Copy link
Contributor Author

XComp commented Sep 27, 2024 •
edited
Loading

Most of the Scala code is gone in master now which unblocked this PR (I had an issue with the Scala compiler fixing one of the windowAll comments and I didn't want to bother to fix it considering that we were planning to remove the Scala code). I rebased the branch and addressed all comments.

@XComp
Copy link
Contributor Author

XComp commented Sep 27, 2024 •
edited
Loading

Failure related to FLINK-36356.

@XComp
Copy link
Contributor Author

XComp commented Sep 27, 2024

@flinkbot run azure

zentol
zentol reviewed Sep 27, 2024
View reviewed changes
README.md Show resolved Hide resolved
zentol
zentol approved these changes Sep 27, 2024
View reviewed changes
Copy link
Contributor

@zentol zentol left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Bit irritated that we still got scala examples in the docs, but it's not a problem for this PR I suppose.

@XComp
Copy link
Contributor Author

XComp commented Sep 27, 2024

Bit irritated that we still got scala examples in the docs, but it's not a problem for this PR I suppose.

Looks like there is a separate issue FLINK-36352 for that.

@XComp
Copy link
Contributor Author

XComp commented Sep 27, 2024

@flinkbot run azure

1 similar comment
@XComp
Copy link
Contributor Author

XComp commented Sep 28, 2024

@flinkbot run azure

@XComp
Copy link
Contributor Author

XComp commented Sep 29, 2024

Something's odd with the cibot. But GHA had a successful CI run on this PR/commit: https://github.com/XComp/flink/actions/runs/11091729815

I will merge the PR. Thanks for the approval

@XComp XComp merged commit 502d967 into apache:master Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reswqa reswqa reswqa left review comments

@zentol zentol zentol approved these changes

Assignees

@zentol zentol

Labels
component=API/DataStream component=Runtime/Configuration component=Runtime/Coordination
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@XComp @flinkbot @zentol @reswqa