zentol commented on code in PR #21012:
URL: https://github.com/apache/flink/pull/21012#discussion_r1015217097
##########
flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/JarRunHandler.java:
##########
@@ -130,24 +130,29 @@ private SavepointRestoreSettings
getSavepointRestoreSettings(
final JarRunRequestBody requestBody = request.getRequestBody();
+ Configuration configuration = requestBody.getFlinkConfiguration();
Review Comment:
why isn't this using the effective configuration?
##########
flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/handlers/JarHandlerParameterTest.java:
##########
@@ -68,6 +72,17 @@ enum ProgramArgsParType {
static final String[] PROG_ARGS = new String[] {"--host", "localhost",
"--port", "1234"};
static final int PARALLELISM = 4;
+ static final String RESTORE_PATH = "/foo/bar";
+ static final boolean ALLOW_NON_RESTORED_STATE_QUERY = true;
+ static final RestoreMode RESTORE_MODE = RestoreMode.CLAIM;
+
+ static final Map<String, String> FLINK_CONFIGURATION =
Review Comment:
Construct a `Configuration` and convert it to a map as needed instead.
##########
flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/handlers/JarRunRequestBodyTest.java:
##########
@@ -59,5 +63,11 @@ protected void assertOriginalEqualsToUnmarshalled(
.isEqualTo(expected.getAllowNonRestoredState());
assertThat(actual.getSavepointPath()).isEqualTo(expected.getSavepointPath());
assertThat(actual.getRestoreMode()).isEqualTo(expected.getRestoreMode());
+ assertThat(
+ Maps.difference(
+
expected.getFlinkConfiguration().toMap(),
+ actual.getFlinkConfiguration().toMap())
+ .areEqual())
+ .isEqualTo(true);
Review Comment:
```suggestion
assertThat(actual.getFlinkConfiguration().toMap())
.containsExactlyEntriesOf(expected.getFlinkConfiguration().toMap());
```
##########
flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/handlers/JarHandlerParameterTest.java:
##########
@@ -69,6 +71,11 @@ enum ProgramArgsParType {
static final String[] PROG_ARGS = new String[] {"--host", "localhost",
"--port", "1234"};
static final int PARALLELISM = 4;
+ static final Map<String, String> FLINK_CONFIGURATION =
+ ImmutableMap.of(
+ CoreOptions.DEFAULT_PARALLELISM.key(), "2",
Review Comment:
> when we run JarRunHandler in the code, the above value will still appear
in flinkconfig
I don't get it. We are in full control in these tests as to whether the
configuration is passed via the request body and what "flink configuration" is
passed to the handler at construction time.
Why would it _impossible_ to test?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
