You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "byronellis (via GitHub)" <gi...@apache.org> on 2023/08/03 23:43:20 UTC
[GitHub] [beam] byronellis opened a new pull request, #27841: Add PIPELINE_OPTIONS_FILE environment variable to allow PIPELINE_OPTIONS to be read from a file
byronellis opened a new pull request, #27841:
URL: https://github.com/apache/beam/pull/27841
Add an option to the FnApi harness PIPELINE_OPTIONS_FILE that overrides the old PIPELINE_OPTIONS environment variable with the contents of a file. This allows us to circumvent kernel limits on the combined size of environment variables and commandl ine arguments.
Addresses #27839
**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:
- [ ] 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)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](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 or the [workflows README](https://github.com/apache/beam/blob/master/.github/workflows/README.md) to see a list of phrases to trigger workflows.
--
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 #27841: Add PIPELINE_OPTIONS_FILE environment variable to allow PIPELINE_OPTIONS to be read from a file
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #27841:
URL: https://github.com/apache/beam/pull/27841#issuecomment-1665965247
GitHub Action failures are due to #27854 , not related
--
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] lostluck commented on pull request #27841: Add PIPELINE_OPTIONS_FILE environment variable to allow PIPELINE_OPTIONS to be read from a file
Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck commented on PR #27841:
URL: https://github.com/apache/beam/pull/27841#issuecomment-1668316368
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 pull request #27841: Add PIPELINE_OPTIONS_FILE environment variable to allow PIPELINE_OPTIONS to be read from a file
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #27841:
URL: https://github.com/apache/beam/pull/27841#issuecomment-1665964438
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] lostluck merged pull request #27841: Add PIPELINE_OPTIONS_FILE environment variable to allow PIPELINE_OPTIONS to be read from a file
Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck merged PR #27841:
URL: https://github.com/apache/beam/pull/27841
--
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] lostluck commented on pull request #27841: Add PIPELINE_OPTIONS_FILE environment variable to allow PIPELINE_OPTIONS to be read from a file
Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck commented on PR #27841:
URL: https://github.com/apache/beam/pull/27841#issuecomment-1664801911
I'm of the opinion that most of the failures are unrelated.
--
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] lostluck commented on pull request #27841: Add PIPELINE_OPTIONS_FILE environment variable to allow PIPELINE_OPTIONS to be read from a file
Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck commented on PR #27841:
URL: https://github.com/apache/beam/pull/27841#issuecomment-1668357785
Run Dataflow ValidatesRunner
--
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] bvolpato commented on a diff in pull request #27841: Add PIPELINE_OPTIONS_FILE environment variable to allow PIPELINE_OPTIONS to be read from a file
Posted by "bvolpato (via GitHub)" <gi...@apache.org>.
bvolpato commented on code in PR #27841:
URL: https://github.com/apache/beam/pull/27841#discussion_r1287156098
##########
sdks/java/harness/src/main/java/org/apache/beam/fn/harness/FnHarness.java:
##########
@@ -117,11 +123,29 @@ public static void main(Function<String, String> environmentVarGetter) throws Ex
"Control location %s%n", environmentVarGetter.apply(CONTROL_API_SERVICE_DESCRIPTOR));
System.out.format(
"Status location %s%n", environmentVarGetter.apply(STATUS_API_SERVICE_DESCRIPTOR));
- System.out.format("Pipeline options %s%n", environmentVarGetter.apply(PIPELINE_OPTIONS));
-
String id = environmentVarGetter.apply(HARNESS_ID);
- PipelineOptions options =
- PipelineOptionsTranslation.fromJson(environmentVarGetter.apply(PIPELINE_OPTIONS));
+
+ String pipelineOptionsJson = environmentVarGetter.apply(PIPELINE_OPTIONS);
+ // Try looking for a file first. If that exists it should override PIPELINE_OPTIONS to avoid
+ // maxing out the kernel's environment space
+ try {
+ String pipelineOptionsPath = environmentVarGetter.apply(PIPELINE_OPTIONS_FILE);
+ System.out.format("Pipeline Options File %s%n", pipelineOptionsPath);
+ if (pipelineOptionsPath != null) {
+ Path filePath = Paths.get(pipelineOptionsPath);
+ if (Files.exists(filePath)) {
+ System.out.format(
+ "Pipeline Options File %s exists. Overriding existing options.%n",
+ pipelineOptionsPath);
+ pipelineOptionsJson = new String(Files.readAllBytes(filePath), StandardCharsets.UTF_8);
+ }
+ }
+ } catch (Exception e) {
+ System.out.format("Problem loading pipeline options from file: %s%n", e.getMessage());
+ }
+
+ System.out.format("Pipeline options %s%n", pipelineOptionsJson);
Review Comment:
Nevermind, I see it just continued the trend from before.
--
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] bvolpato commented on a diff in pull request #27841: Add PIPELINE_OPTIONS_FILE environment variable to allow PIPELINE_OPTIONS to be read from a file
Posted by "bvolpato (via GitHub)" <gi...@apache.org>.
bvolpato commented on code in PR #27841:
URL: https://github.com/apache/beam/pull/27841#discussion_r1287151799
##########
sdks/java/harness/src/main/java/org/apache/beam/fn/harness/FnHarness.java:
##########
@@ -117,11 +123,29 @@ public static void main(Function<String, String> environmentVarGetter) throws Ex
"Control location %s%n", environmentVarGetter.apply(CONTROL_API_SERVICE_DESCRIPTOR));
System.out.format(
"Status location %s%n", environmentVarGetter.apply(STATUS_API_SERVICE_DESCRIPTOR));
- System.out.format("Pipeline options %s%n", environmentVarGetter.apply(PIPELINE_OPTIONS));
-
String id = environmentVarGetter.apply(HARNESS_ID);
- PipelineOptions options =
- PipelineOptionsTranslation.fromJson(environmentVarGetter.apply(PIPELINE_OPTIONS));
+
+ String pipelineOptionsJson = environmentVarGetter.apply(PIPELINE_OPTIONS);
+ // Try looking for a file first. If that exists it should override PIPELINE_OPTIONS to avoid
+ // maxing out the kernel's environment space
+ try {
+ String pipelineOptionsPath = environmentVarGetter.apply(PIPELINE_OPTIONS_FILE);
+ System.out.format("Pipeline Options File %s%n", pipelineOptionsPath);
+ if (pipelineOptionsPath != null) {
+ Path filePath = Paths.get(pipelineOptionsPath);
+ if (Files.exists(filePath)) {
+ System.out.format(
+ "Pipeline Options File %s exists. Overriding existing options.%n",
+ pipelineOptionsPath);
+ pipelineOptionsJson = new String(Files.readAllBytes(filePath), StandardCharsets.UTF_8);
+ }
+ }
+ } catch (Exception e) {
+ System.out.format("Problem loading pipeline options from file: %s%n", e.getMessage());
+ }
+
+ System.out.format("Pipeline options %s%n", pipelineOptionsJson);
Review Comment:
Intentionally not using logger?
--
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 #27841: Add PIPELINE_OPTIONS_FILE environment variable to allow PIPELINE_OPTIONS to be read from a file
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #27841:
URL: https://github.com/apache/beam/pull/27841#issuecomment-1664847970
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`
--
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] lostluck commented on pull request #27841: Add PIPELINE_OPTIONS_FILE environment variable to allow PIPELINE_OPTIONS to be read from a file
Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck commented on PR #27841:
URL: https://github.com/apache/beam/pull/27841#issuecomment-1668463431
Run Java Dataflow V2 ValidatesRunner
--
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] lostluck commented on pull request #27841: Add PIPELINE_OPTIONS_FILE environment variable to allow PIPELINE_OPTIONS to be read from a file
Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck commented on PR #27841:
URL: https://github.com/apache/beam/pull/27841#issuecomment-1668463106
Run PortableJar_Flink 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] lostluck commented on pull request #27841: Add PIPELINE_OPTIONS_FILE environment variable to allow PIPELINE_OPTIONS to be read from a file
Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck commented on PR #27841:
URL: https://github.com/apache/beam/pull/27841#issuecomment-1666084874
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] lostluck commented on a diff in pull request #27841: Add PIPELINE_OPTIONS_FILE environment variable to allow PIPELINE_OPTIONS to be read from a file
Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck commented on code in PR #27841:
URL: https://github.com/apache/beam/pull/27841#discussion_r1283826990
##########
sdks/java/harness/src/main/java/org/apache/beam/fn/harness/FnHarness.java:
##########
@@ -117,11 +123,29 @@ public static void main(Function<String, String> environmentVarGetter) throws Ex
"Control location %s%n", environmentVarGetter.apply(CONTROL_API_SERVICE_DESCRIPTOR));
System.out.format(
"Status location %s%n", environmentVarGetter.apply(STATUS_API_SERVICE_DESCRIPTOR));
- System.out.format("Pipeline options %s%n", environmentVarGetter.apply(PIPELINE_OPTIONS));
-
String id = environmentVarGetter.apply(HARNESS_ID);
- PipelineOptions options =
- PipelineOptionsTranslation.fromJson(environmentVarGetter.apply(PIPELINE_OPTIONS));
+
+ String pipelineOptionsJson = environmentVarGetter.apply(PIPELINE_OPTIONS);
+ // Try looking for a file first. If that exists it should override PIPELINE_OPTIONS to avoid
Review Comment:
Technically we're not looking for the file first, so I say the comment needs adjusting. Otherwise LGTM.
--
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