kennknowles commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r998897966
##########
sdks/python/test-suites/portable/common.gradle:
##########
@@ -304,6 +304,13 @@
project.tasks.register("postCommitPy${pythonVersionSuffix}IT") {
"--environment_type=LOOPBACK",
"--temp_location=gs://temp-storage-for-end-to-end-tests/temp-it",
"--flink_job_server_jar=${project(":runners:flink:${latestFlinkVersion}:job-server").shadowJar.archivePath}",
+ '--sdk_harness_log_level_overrides=' +
+ // suppress info level flink.runtime log flood
+ '{\\"org.apache.flink.runtime\\":\\"WARN\\",' +
Review Comment:
This basically looks funny to me because the SDK harness is only actually
used in portable mode, where there is no `org.apache.flink` namespace.
So the thing we are changing is when it is run in non-portable mode, and
there is not actually any SDK harness.
I don't know a good solution. It is just a naming thing. Probably good to
have a single flag that works now and also later. Is there not already a
`--log_level_overrides` ? I guess then it is ambiguous whether you are applying
it to the runner or to the SDK harness.
I have no solution. This is basically OK with me, but something about it is
not perfectly clean.
Maybe @lukecwik has an opinion about which flags should control which log
levels. I know that today we have
1. Flag for the Dataflow worker
2. Flag for the SDK harness
So unfortunately neither are a good choice for sharing with other runners.
I don't know if any other runner has any flag at all. My issue is that "SDK
harness" is a pretty weird name for a concept that most users really don't or
shouldn't care about or know exists most of the time.
##########
runners/flink/src/main/java/org/apache/beam/runners/flink/FlinkPipelineRunner.java:
##########
@@ -67,14 +68,18 @@ public class FlinkPipelineRunner implements
PortablePipelineRunner {
public FlinkPipelineRunner(
FlinkPipelineOptions pipelineOptions, @Nullable String confDir,
List<String> filesToStage) {
this.pipelineOptions = pipelineOptions;
- this.confDir = confDir;
+ // confDir takes precedence than pipelineOptions.getFlinkConfDir
Review Comment:
Put this in the javadoc. A user needs to know.
--
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]
