Skip to content

[FLINK-34083][config] Deprecate string configuration keys and unused constants in ConfigConstants#24089

Merged
1996fanrui merged 2 commits intoapache:masterfrom
Sxnan:FLINK-34083
Jan 19, 2024
Merged

[FLINK-34083][config] Deprecate string configuration keys and unused constants in ConfigConstants#24089
1996fanrui merged 2 commits intoapache:masterfrom
Sxnan:FLINK-34083

Conversation

@Sxnan
Copy link
Copy Markdown
Contributor

@Sxnan Sxnan commented Jan 15, 2024

What is the purpose of the change

This PR updates ConfigConstants.java to deprecate and replace string configuration keys and marks unused constants in ConfigConstants.java as deprecated.

Brief change log

  • Move BootstrapTools#getTaskManagerShellCommand to flink-yarn Utils
  • Deprecate string configuration keys and unused constants in ConfigConstants

Verifying this change

This change is already covered by existing tests.

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): yes
  • 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

@1996fanrui 1996fanrui self-requested a review January 15, 2024 08:18
@flinkbot
Copy link
Copy Markdown
Collaborator

flinkbot commented Jan 15, 2024

CI report:

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

@Sxnan Sxnan force-pushed the FLINK-34083 branch 3 times, most recently from adac29f to b6e61ef Compare January 16, 2024 06:04
@Sxnan Sxnan marked this pull request as ready for review January 16, 2024 10:36
@Sxnan
Copy link
Copy Markdown
Contributor Author

Sxnan commented Jan 17, 2024

@1996fanrui @xintongsong Could you review this PR?

Copy link
Copy Markdown
Member

@1996fanrui 1996fanrui left a comment

Choose a reason for hiding this comment

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

Thanks @Sxnan for the contribution!

I left 2 minor comments, please take a look in your free time.

BTW, IIUC the first commit is a refactor without any real change, right? If so, it's better to add [refactor] in the commit message. It's helpful for understand these commit, thanks~

Copy link
Copy Markdown
Contributor

@xintongsong xintongsong left a comment

Choose a reason for hiding this comment

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

LGTM.

@Sxnan
Copy link
Copy Markdown
Contributor Author

Sxnan commented Jan 18, 2024

@flinkbot run azure

1 similar comment
@1996fanrui
Copy link
Copy Markdown
Member

@flinkbot run azure

@1996fanrui
Copy link
Copy Markdown
Member

The CI is green, merging~

@1996fanrui 1996fanrui merged commit 38f7b51 into apache:master Jan 19, 2024
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.

4 participants