Skip to content

Flink: Fix checkArgument message for flink streaming#15907

Open
genxiong7 wants to merge 1 commit intoapache:mainfrom
genxiong7:flink_message_fix
Open

Flink: Fix checkArgument message for flink streaming#15907
genxiong7 wants to merge 1 commit intoapache:mainfrom
genxiong7:flink_message_fix

Conversation

@genxiong7
Copy link
Copy Markdown

@genxiong7 genxiong7 commented Apr 7, 2026

This is a small follow-up fix for the Flink streaming error message. The change is limited to the message text.

Copy link
Copy Markdown
Contributor

@mxm mxm left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: The checkArgument method supports a string template, so there's no need to use String.format on the caller side:

checkArgument(boolean expression, String errorMessageTemplate, @Nullable Object... errorMessageArgs)

Please feel free to ignore this comment since it's unrelated to the error message change.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. I’ve updated it accordingly.

@genxiong7 genxiong7 force-pushed the flink_message_fix branch 2 times, most recently from 7733176 to d92dd5f Compare April 8, 2026 14:56
Preconditions.checkArgument(
startSnapshotId == null,
"Invalid starting snapshot id for SPECIFIC_START_SNAPSHOT_ID strategy: not null");
"Invalid starting snapshot id for SPECIFIC_START_SNAPSHOT_TIMESTAMP strategy: not null");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
"Invalid starting snapshot id for SPECIFIC_START_SNAPSHOT_TIMESTAMP strategy: not null");
"Invalid starting snapshot id for %s strategy: not null", startingStrategy);

Let's interpolate the string to prevent copy/paste issues in the future.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Makes sense. Fixed by interpolating the string, and also corrected the mismatch between the error message and the strategy name.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants