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-11643] remove useless code of externalizedCheckpointsDir #7729
Conversation
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. Bot commandsThe @flinkbot bot supports the following commands:
|
|
||
switch (backendName.toLowerCase()) { | ||
case MEMORY_STATE_BACKEND_NAME: | ||
MemoryStateBackend memBackend = new MemoryStateBackendFactory().createFromConfig(config); | ||
|
||
if (logger != null) { | ||
if (logIsNotNull) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you have changed this, should we add a test logic that is info log enabled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a minor change, i will add a test logic as you suggested, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest to revert the introduction of logIsNotNull
. What is the benefit of the change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just think logIsNotNull
is more readable as there are many logger != null
logic in the method. Ok, i will revert this point if it make no sense.
f56dcaf
to
e4c6ee8
Compare
No longer applicable. |
What is the purpose of the change
Remove unless code of
externalizedCheckpointsDir
, besides make tiny changes toStateBackendLoader.java
to be more readableBrief change log
externalizedCheckpointsDir
and make some tiny changes in classStateBackendLoader
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (no)Documentation