You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/10/14 02:03:29 UTC

[GitHub] [beam] Abacn opened a new pull request, #23635: Portable log overrides PoC

Abacn opened a new pull request, #23635:
URL: https://github.com/apache/beam/pull/23635

   * Flink runner support log_level_overrides
   
   * Decrease Flink Runner Log Spam for Python PostCommit
   
   **Please** add a meaningful description for your change here
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
    - [ ] Update `CHANGES.md` with noteworthy changes.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/get-started-contributing/#make-the-reviewers-job-easier).
   
   To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Go tests](https://github.com/apache/beam/workflows/Go%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
   
   See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] kennknowles commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r999840416


##########
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:
   Thanks, please update the Javadoc as well.



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r999652302


##########
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:
   Yes, we have two flags about log level overrides, another one is in DataflowWorkerLoggingOptions but this option class is deprecated.



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn merged pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
tvalentyn merged PR #23635:
URL: https://github.com/apache/beam/pull/23635


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1283173520

   Java precommit fail due to flakes: #21480 #21714
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1282361101

   Run SQL_Java17 PreCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r999761017


##########
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:
   If we decide to create new options for log overrides I think it would be under another Issue.



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] codecov[bot] commented on pull request #23635: Portable log overrides PoC

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1278400907

   # [Codecov](https://codecov.io/gh/apache/beam/pull/23635?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#23635](https://codecov.io/gh/apache/beam/pull/23635?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ce9a0a0) into [master](https://codecov.io/gh/apache/beam/commit/98cebe7b1255563c14846a999b81c386d25d4c58?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (98cebe7) will **increase** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head ce9a0a0 differs from pull request most recent head 3c40cda. Consider uploading reports for the commit 3c40cda to get more accurate results
   
   ```diff
   @@           Coverage Diff           @@
   ##           master   #23635   +/-   ##
   =======================================
     Coverage   73.32%   73.33%           
   =======================================
     Files         719      719           
     Lines       95792    95791    -1     
   =======================================
   + Hits        70241    70247    +6     
   + Misses      24240    24233    -7     
     Partials     1311     1311           
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `83.05% <100.00%> (+0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/23635?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...dks/python/apache\_beam/options/pipeline\_options.py](https://codecov.io/gh/apache/beam/pull/23635/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vb3B0aW9ucy9waXBlbGluZV9vcHRpb25zLnB5) | `94.43% <100.00%> (+0.06%)` | :arrow_up: |
   | [...thon/apache\_beam/runners/worker/sdk\_worker\_main.py](https://codecov.io/gh/apache/beam/pull/23635/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlcl9tYWluLnB5) | `77.71% <100.00%> (-0.78%)` | :arrow_down: |
   | [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/23635/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.42% <0.00%> (-0.13%)` | :arrow_down: |
   | [sdks/python/apache\_beam/runners/common.py](https://codecov.io/gh/apache/beam/pull/23635/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9jb21tb24ucHk=) | `88.59% <0.00%> (-0.13%)` | :arrow_down: |
   | [...eam/runners/portability/fn\_api\_runner/execution.py](https://codecov.io/gh/apache/beam/pull/23635/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2V4ZWN1dGlvbi5weQ==) | `93.08% <0.00%> (+0.64%)` | :arrow_up: |
   | [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/23635/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9ydW5uZXIucHk=) | `91.77% <0.00%> (+1.26%)` | :arrow_up: |
   | [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/23635/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `100.00% <0.00%> (+1.61%)` | :arrow_up: |
   | [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/23635/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdGVzdGluZy90ZXN0X3N0cmVhbV9zZXJ2aWNlLnB5) | `92.85% <0.00%> (+4.76%)` | :arrow_up: |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Portable log overrides PoC

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1281593248

   Run Python 3.8 PostCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r998480542


##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -448,6 +451,39 @@ public static String getExternalServiceAddress(PortablePipelineOptions options)
     return environmentOption;
   }
 
+  /**
+   * Configure log manager's default log level and log level overrides from the sdk harness options,
+   * and return the list of configured loggers.
+   */
+  public static List<java.util.logging.Logger> getConfiguredLoggerFromOptions(

Review Comment:
   Was trying to find the best place to put these piece of code that sets log levels. It is now used by both `:sdks:java:harness` and `:runners:flink`. Seems like this module is a good place as there are other static public methods related to environments here.



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r998695121


##########
runners/flink/src/main/java/org/apache/beam/runners/flink/FlinkPipelineRunner.java:
##########
@@ -67,14 +69,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
+    this.confDir = confDir != null ? confDir : pipelineOptions.getFlinkConfDir();
     this.filesToStage = filesToStage;
   }
 
   @Override
   public PortablePipelineResult run(final Pipeline pipeline, JobInfo jobInfo) throws Exception {
     MetricsEnvironment.setMetricsSupported(false);
 
+    // Apply log levels settings at the beginning of pipeline run

Review Comment:
   Yeah, not yet put it in because haven't tested it on spark runner. Entered #23713



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Portable log overrides PoC

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1281669988

   Run Python 3.8 PostCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] AnandInguva commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1284575092

   >>  wow all Jenkins tests passed once. Rare
   
   Is the PreCommit failure on Python 3.10 flaky?
   
   
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1284448868

   > Python PreCommit fails broken pipe on newly added Py310 test
   
   It's being worked on: https://github.com/apache/beam/issues/23734


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1282363379

   R: @tvalentyn 


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r998689685


##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -448,6 +451,39 @@ public static String getExternalServiceAddress(PortablePipelineOptions options)
     return environmentOption;
   }
 
+  /**
+   * Configure log manager's default log level and log level overrides from the sdk harness options,
+   * and return the list of configured loggers.
+   */
+  public static List<java.util.logging.Logger> getConfiguredLoggerFromOptions(

Review Comment:
   is `SdkHarnessOptions.java` accessible from `:runners:flink` ?



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r999761017


##########
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:
   If we decide to create new options for log overrides I think it would be under another issue.



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1282633584

   Run Java_Spark3_Versions PreCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1281789577

   Run Python 3.8 PostCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Portable log overrides PoC

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1281759980

   Run Python 3.8 PostCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1282360490

   Run Java PreCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r999815001


##########
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:
   At a first glance I understood `confDir` parameter as an override to `pipelineOptions.getFlinkConfDir` in `FlinkPipelineRunner's` constructor.
   
   One use case is that the command lines argument of
   https://github.com/apache/beam/blob/3edff6a7cf3c57e4589837f2714dbdd981a32063/runners/flink/src/main/java/org/apache/beam/runners/flink/FlinkPipelineRunner.java#L160
   could pass to `FlinkPipelineRunnerConfiguration ` and finally appears as `confDir` parameter here. According to  the comment of https://github.com/apache/beam/blob/3edff6a7cf3c57e4589837f2714dbdd981a32063/runners/flink/src/main/java/org/apache/beam/runners/flink/FlinkPipelineRunner.java#L213
   I understand that if `confDir` is set it should take precedence. 
   
   
   



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1282360848

   Run Typescript PreCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r998690891


##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -448,6 +451,39 @@ public static String getExternalServiceAddress(PortablePipelineOptions options)
     return environmentOption;
   }
 
+  /**
+   * Configure log manager's default log level and log level overrides from the sdk harness options,
+   * and return the list of configured loggers.
+   */
+  public static List<java.util.logging.Logger> getConfiguredLoggerFromOptions(

Review Comment:
   have you considered it as a home for this helper?



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r999964205


##########
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:
   I see, then this is in portable mode confDir settings in pipeline options did not take effect.



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #23635: Portable log overrides PoC

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r997260730


##########
runners/flink/src/main/java/org/apache/beam/runners/flink/FlinkExecutionEnvironments.java:
##########
@@ -73,7 +74,7 @@ public static ExecutionEnvironment createBatchExecutionEnvironment(FlinkPipeline
   static ExecutionEnvironment createBatchExecutionEnvironment(
       FlinkPipelineOptions options, List<String> filesToStage, @Nullable String confDir) {
 
-    LOG.info("Creating a Batch Execution Environment.");
+    LOG.info("Creating a Batch Execution Environment with config {}.", confDir);

Review Comment:
   Most log spams comes from `org.apache.flink.runtime` during flink minicluster initiating. Tested that if assign confDir to a directory that has org.apache.flink.runtime WARN logging, it works. However, currently this parameter is always null (seems like beam not respecting parsed in flinkConfDir)



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Portable log overrides PoC

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1281123188

   Run Python 3.8 PostCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Portable log overrides PoC

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1278386437

   Run Python 3.8 PostCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r998695661


##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -448,6 +451,39 @@ public static String getExternalServiceAddress(PortablePipelineOptions options)
     return environmentOption;
   }
 
+  /**
+   * Configure log manager's default log level and log level overrides from the sdk harness options,
+   * and return the list of configured loggers.
+   */
+  public static List<java.util.logging.Logger> getConfiguredLoggerFromOptions(

Review Comment:
   SdkHarnessOptions.java is in sdks:java:core and yes it is accessible. Considered it and I was not sure if it is a good pattern to include some code that have side effect on the environment in the option class. If it sounds reasonable could move it in.



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1281805018

   Run Python 3.8 PostCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1282356726

   Logs reduced from ~60 MB to 10 MB: https://ci-beam.apache.org/job/beam_PostCommit_Python38_PR/653/console


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Portable log overrides PoC

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1278400472

   Run Python 3.8 PostCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1284609035

   > 
   
   It is considered flaky if not fixed in between my second last (failure) and the last push (suceeded).


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r998838083


##########
sdks/java/core/src/main/java/org/apache/beam/sdk/options/SdkHarnessOptions.java:
##########
@@ -344,4 +347,36 @@ public static SdkHarnessLogLevelOverrides from(Map<String, String> values) {
   List<String> getJdkAddOpenModules();
 
   void setJdkAddOpenModules(List<String> options);
+
+  /**
+   * Configure log manager's default log level and log level overrides from the sdk harness options,
+   * and return the list of configured loggers.
+   */
+  static List<Logger> getConfiguredLoggerFromOptions(SdkHarnessOptions loggingOptions) {
+    ArrayList<Logger> configuredLoggers = new ArrayList<>();
+    LogManager logManager = LogManager.getLogManager();
+    Logger rootLogger = logManager.getLogger("");
+
+    // Use the passed in logging options to configure the various logger levels.
+    if (loggingOptions.getDefaultSdkHarnessLogLevel() != null) {
+      rootLogger.setLevel(

Review Comment:
   from reading this method it's not clear why rootLogger is not included into the configuredLoggers. should we modify the root logger separately, for example where this method is called?



##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -448,6 +451,39 @@ public static String getExternalServiceAddress(PortablePipelineOptions options)
     return environmentOption;
   }
 
+  /**
+   * Configure log manager's default log level and log level overrides from the sdk harness options,
+   * and return the list of configured loggers.
+   */
+  public static List<java.util.logging.Logger> getConfiguredLoggerFromOptions(

Review Comment:
   Took another look, agree that it is looks somewhat unnatural to modify the global context, especially the root logger. We can ask for second opinion on this from someone who does more work on Java SDK. 
   @kennknowles  might have good advice. 
   



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1284438125

   Python Commit fails brocken pipe on newly added Py310 test
   
   ```
   Failed
   pytest.internal (from preCommitIT-df-py310)
   
   Failing for the past 1 build (Since #25360 )
   Took 0 ms.
   Error Message
   internal error
   ```


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1284438816

   Java PreCommit fails known flakes
   ```
   org.apache.beam.runners.fnexecution.control.RemoteExecutionTest.testSplit
   org.apache.beam.sdk.io.pulsar.PulsarIOTest.classMethod
   ```


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1284556945

   wow all Jenkins tests passed once. Rare


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r998690710


##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -448,6 +451,39 @@ public static String getExternalServiceAddress(PortablePipelineOptions options)
     return environmentOption;
   }
 
+  /**
+   * Configure log manager's default log level and log level overrides from the sdk harness options,
+   * and return the list of configured loggers.
+   */
+  public static List<java.util.logging.Logger> getConfiguredLoggerFromOptions(

Review Comment:
   I see you do import that file in FlinkPipelineRunner.java so it must be



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1282361673

   Run Java_Kafka_IO_Direct PreCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r998682297


##########
runners/flink/src/main/java/org/apache/beam/runners/flink/FlinkPipelineRunner.java:
##########
@@ -67,14 +69,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
+    this.confDir = confDir != null ? confDir : pipelineOptions.getFlinkConfDir();
     this.filesToStage = filesToStage;
   }
 
   @Override
   public PortablePipelineResult run(final Pipeline pipeline, JobInfo jobInfo) throws Exception {
     MetricsEnvironment.setMetricsSupported(false);
 
+    // Apply log levels settings at the beginning of pipeline run

Review Comment:
   do we need a similar change in https://github.com/apache/beam/blob/master/runners/spark/src/main/java/org/apache/beam/runners/spark/SparkPipelineRunner.java ? Can be  a separate PR.



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1282362645

   Rerun failed tests. These fail are mostly due to infrastructure flakes, eg timeout/gradle daemon things.


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1283173654

   Run Java_Spark3_Versions PreCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Portable log overrides PoC

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1278377566

   Run Python 3.8 PostCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r999830285


##########
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:
   Thanks! That makes sense to me. confDir is per server and pipelineOptions is per pipeline. Will change.



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1284808402

   Thanks, @Abacn !


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Portable log overrides PoC

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1281629889

   Run Python 3.8 PostCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] github-actions[bot] commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1282488723

   Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #23635:
URL: https://github.com/apache/beam/pull/23635#issuecomment-1282361373

   Run Python PreCommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r999844890


##########
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:
   I observe the option does not take effect when invoking a Python pipeline and I targeted the code here. It may be related to What @kennknowles mentioned below there is a "non-portable mode" and also a "portable mode" having different entry points for environment settings.



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r999652302


##########
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:
   Yes, we have two flags about log level overrides, another one is in DataflowWorkerLoggingOptions but this option class is deprecated. So I just use the existing `sdk_harness_log_level_overrides ` unless we want to create another option flag. I agree `--log_level_overrides` would make generic sense.



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r999661677


##########
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:
   Thanks, done. This is a fix, originally pipelineOptions's flinkConfDir did not take effect at all.



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r999955090


##########
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:
   Running Python pipelines on Flink Runner should always be a portable mode.



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r999798980


##########
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:
   Shouldn't pipeline option take precedence though?



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r998695661


##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -448,6 +451,39 @@ public static String getExternalServiceAddress(PortablePipelineOptions options)
     return environmentOption;
   }
 
+  /**
+   * Configure log manager's default log level and log level overrides from the sdk harness options,
+   * and return the list of configured loggers.
+   */
+  public static List<java.util.logging.Logger> getConfiguredLoggerFromOptions(

Review Comment:
   SdkHarnessOptions.java is in sdks:java:core and yes it is accessible



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on a diff in pull request #23635: Reduce log flood in Python PostCommit flink task

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #23635:
URL: https://github.com/apache/beam/pull/23635#discussion_r999828902


##########
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:
   Found  origins of this flag: 
   From: https://issues.apache.org/jira/browse/BEAM-14492 
   ```
   Sometimes it is necessary to be able to set any flink option via PipelineOptions to the runner - mostly when we submit job from vanilla Java, not being run via flink run
   ```
   https://github.com/apache/beam/pull/17715
   
   I think the intent is consistent with the usage that if a user specifies this via pipeline options, it should take precedence over predetermined runner configuration. 
   cc'ing @je-ik just in case and FYI since you mention that ```originally pipelineOptions's flinkConfDir did not take effect at all.```



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org