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

[hotfix][docs] Fix scan.bounded.mode missing forward options #60

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Tan-JiaLiang
Copy link

@Tan-JiaLiang Tan-JiaLiang commented Oct 13, 2023

  1. add scan.bounded.mode, scan.startup.specific-offsets, scan.bounded.specific-offsets to the forward options.
  2. remove sink.parallelism from the forward options.
  3. fix the scan.bounded.mode docs.
  4. improve Chinese docs.

Copy link
Contributor

@MartijnVisser MartijnVisser left a comment

Choose a reason for hiding this comment

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

@Tan-JiaLiang Thanks for the PR. Do we have tests for them? And can you also check the Chinese docs?

@MartijnVisser MartijnVisser self-assigned this Jan 18, 2024
@Tan-JiaLiang
Copy link
Author

@MartijnVisser Sorry late.

Do we have tests for them?

To be honest, I checked all connectors(e.g. jdbc/hbase/filesystem...) but there is no test case for forward options.

Forward options is a set of ConfigOption that are directly forwarded to the runtime implementation but don't affect the final execution topology. And according to the design of FLINK-24456, scan.bounded.mode, scan.bounded.specific-offsets, scan.bounded.timestamp-millis should be forward options.

And can you also check the Chinese docs?

Yes, I filled in the gaps in the Chinese documentation and translated the content.

Finally, the sink.parallelism option should not be a forward option, as it can lead to inconsistent parallelism between upstream and downstream operators and change the execution topology.

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

Successfully merging this pull request may close these issues.

2 participants