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.

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

[FLINK-27890][examples] Fix the bug introduced By [FLINK-19317] to use the return result after assignTimestampsAndWatermarks #19924

Closed
wants to merge 1 commit into from

Conversation

coderappee
Copy link
Contributor

What is the purpose of the change

Fix the SideOutputExample bug introduced by [FLINK-19317].

Brief change log

  • Use the return result after assign the TimestampsAndWatermarks for the next step

Verifying this change

Please make sure both new and modified tests in this PR follows the conventions defined in our code quality guide: https://flink.apache.org/contributing/code-style-and-quality-common.html#testing

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): (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 / docs / JavaDocs / not documented)

@coderappee coderappee changed the title Flink 27890 [FLINK-27890][examples] Fix the bug introduced By [FLINK-19317] to use the return result after assignTimestampsAndWatermarks Jun 9, 2022
@flinkbot
Copy link
Collaborator

flinkbot commented Jun 9, 2022

CI report:

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

@alpinegizmo
Copy link
Contributor

@coderappee You've ended up with two apparently identical commits in this PR. Could you clean that up, please?

@coderappee
Copy link
Contributor Author

@coderappee You've ended up with two apparently identical commits in this PR. Could you clean that up, please?

Could you tell me how to do it? Close this pull request and then roll back the last two commit and recommit again?

@reswqa
Copy link
Member

reswqa commented Jun 10, 2022

@coderappee You've ended up with two apparently identical commits in this PR. Could you clean that up, please?

Could you tell me how to do it? Close this pull request and then roll back the last two commit and recommit again?

Squash the two commits to one, and force push to you own repo branch coderappee:FLINK-27890,it will update this pull request automatically.

@alpinegizmo
Copy link
Contributor

@coderappee Let me know if you're stuck, and I can take care of this when I merge the PR.

In this particular situation, it should be enough to rollback the last commit in your branch and then force push to the branch in your fork that is the basis for this PR.

…e the return result after assignTimestampsAndWatermarks
@coderappee
Copy link
Contributor Author

@coderappee Let me know if you're stuck, and I can take care of this when I merge the PR.

In this particular situation, it should be enough to rollback the last commit in your branch and then force push to the branch in your fork that is the basis for this PR.

I rollback the last commit, then use "git commit --amend" to add some changes to last second commit, and then force push to the branch for this PR.
I do not know it is ok or not.

@coderappee
Copy link
Contributor Author

@flinkbot run azure

Copy link
Contributor

@alpinegizmo alpinegizmo left a comment

Choose a reason for hiding this comment

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

Thank you @coderappee for your contribution!

@alpinegizmo
Copy link
Contributor

Merged in master with a70e704
Merged in release-1.15 with 6fcec2c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants