You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "Mathdee (via GitHub)" <gi...@apache.org> on 2026/01/21 21:34:51 UTC

[PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

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

   **Issue:**
   As described in #37370, running pipelines with large options on Dataflow causes `fork/exec /usr/local/bin/python: argument list too long`.
   This occurs because the bootloader passes the complete JSON config via `PIPELINE_OPTIONS` environment variable --> exceeds the OS `ARG_MAX` limit.
   
   **The Fix:**
   This change uses an identical pattern from the Go SDK (Issue #27839,  Commit e31e885) to Python.
   1. **boot.go(file):** Writing the pipeline options to a temp file (`pipeline_options.json`) and sets the `PIPELINE_OPTIONS_FILE` environment variable.
   2. **sdk_worker_main.py(file):** This then checks for `PIPELINE_OPTIONS_FILE` and loads the configs from the disk if present.
   
   **Outcome:**
   * Fixes #37370 
   * Feature Parity with Java and Go SDK's for handling large pipeline options
   * Verified with unit tests that ensured priority of file-based loading.
   -----
    - [X] 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).
   
   


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "claudevdm (via GitHub)" <gi...@apache.org>.
claudevdm commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3791534059

   @shunping can you take a pass at this since you have more context? 


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3781270203

   ## Summary of Changes
   
   Hello @Mathdee, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
   
   This pull request introduces a robust solution for handling large pipeline options in the Python SDK, which previously led to an 'argument list too long' error due to operating system limitations on environment variable size. By transitioning from direct environment variable passing to a file-based approach, where options are written to a temporary file and its path is referenced, the system can now accommodate extensive configurations without encountering runtime failures. This enhancement ensures greater stability and consistency across SDKs.
   
   ### Highlights
   
   * **Addresses Argument List Too Long Error**: This pull request resolves the `fork/exec /usr/local/bin/python: argument list too long` error that occurs when running Dataflow pipelines with excessively large pipeline options, by preventing the complete JSON config from being passed directly via the `PIPELINE_OPTIONS` environment variable.
   * **File-based Pipeline Options**: Pipeline options are now written to a temporary file named `pipeline_options.json` by the Go bootloader. The path to this file is then passed to the Python SDK worker via the `PIPELINE_OPTIONS_FILE` environment variable.
   * **Python SDK Worker Adaptation**: The Python SDK worker (`sdk_worker_main.py`) has been updated to check for the `PIPELINE_OPTIONS_FILE` environment variable and load the pipeline configuration from the specified file if present, ensuring compatibility with the new mechanism.
   * **Feature Parity**: This change brings the Python SDK into feature parity with the Java and Go SDKs for handling large pipeline options, using an identical pattern previously implemented in the Go SDK.
   
   🧠 **New Feature in Public Preview:** You can now enable **Memory** to help **Gemini Code Assist** learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. **Click [here](https://codeassist.google/code-review/login) to enable Memory in your admin console.**
   
   
   
   
   
   
   
   <details>
   <summary><b>Using Gemini Code Assist</b></summary>
   <br>
   
   The full guide for Gemini Code Assist can be found on our [documentation page](https://developers.google.com/gemini-code-assist/docs/review-github-code), here are some quick tips.
   
   <b>Invoking Gemini</b>
   
   You can request assistance from Gemini at any point by creating a comment using either `/gemini <command>` or `@gemini-code-assist <command>`. Below is a summary of the supported commands on the current page.
   
   Feature | Command | Description
   --- | --- | ---
   Code Review | `/gemini review` | Performs a code review for the current pull request in its current state.
   Pull Request Summary | `/gemini summary` | Provides a summary of the current pull request in its current state.
   Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments.
   Help | `/gemini help` | Displays a list of available commands.
   
   <b>Customization</b>
   
   To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a `.gemini/` folder in the base of the repository. Detailed instructions can be found [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github).
   
   <b>Limitations & Feedback</b>
   
   Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up [here](https://google.qualtrics.com/jfe/form/SV_2cyuGuTWsEw84yG).
   
   <b>You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the [Gemini Code Assist IDE Extension](https://cloud.google.com/products/gemini/code-assist).</b>
   </details>
   
   
   
   
   [^1]: Review the [Privacy Notices](https://policies.google.com/privacy), [Generative AI Prohibited Use Policy](https://policies.google.com/terms/generative-ai/use-policy), [Terms of Service](https://policies.google.com/terms), and learn how to configure Gemini Code Assist in GitHub [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). Gemini can make mistakes, so double check it and [use code with caution](https://support.google.com/legal/answer/13505487).
   


-- 
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

Re: [PR] [Python] Support large pipeline options via file [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3931115637

   Verified the failed workflow [PreCommit Python Integration / beam_PreCommit_Python_Integration](https://github.com/apache/beam/actions/runs/22200230090/job/64210988912?pr=37379) is irrelevant 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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #37379:
URL: https://github.com/apache/beam/pull/37379#discussion_r2737604135


##########
sdks/python/apache_beam/runners/worker/sdk_worker_main.py:
##########
@@ -93,8 +93,25 @@ def create_harness(environment, dry_run=False):
   else:
     fn_log_handler = None
 
-  pipeline_options_dict = _load_pipeline_options(
-      environment.get('PIPELINE_OPTIONS'))
+  options_json = environment.get('PIPELINE_OPTIONS')
+
+  #We check if options are stored in the file.
+  if 'PIPELINE_OPTIONS_FILE' in environment:
+    options_file = environment['PIPELINE_OPTIONS_FILE']
+    try:
+      with open(options_file, 'r') as f:
+        options_json = f.read()
+        _LOGGER.info('Load pipeline options from file: %s', options_file)
+    except:
+      _LOGGER.error(
+          'Failed to load pipeline options from file: %s', options_file)
+      raise
+
+  pipeline_options_dict = _load_pipeline_options(options_json)
+
+  #pipeline_options_dict = _load_pipeline_options(

Review Comment:
   Remove unnecessary comments.



-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3929030944

   @Mathdee: Could you also update CHANGES.md to document this change?
   
   I suggest adding a note to the **'New Features/Improvements'** section regarding the support for large pipeline options via a file. In the **'Breaking Changes'** section, we should also mention that the Python SDK container's `boot.go` now passes pipeline options through a file instead of the `PIPELINE_OPTIONS` environment variable.
   
   Please note that if a user pairs a new Python SDK container with an older SDK version (which does not support the file-based approach), the pipeline options will not be recognized and the pipeline will fail. In that case, should advise users to ensure their SDK and container versions are synchronized.


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #37379:
URL: https://github.com/apache/beam/pull/37379#discussion_r2824456528


##########
sdks/python/apache_beam/runners/worker/sdk_worker_main.py:
##########
@@ -93,8 +93,22 @@ def create_harness(environment, dry_run=False):
   else:
     fn_log_handler = None
 
-  pipeline_options_dict = _load_pipeline_options(
-      environment.get('PIPELINE_OPTIONS'))
+  options_json = environment.get('PIPELINE_OPTIONS')
+
+  #We check if options are stored in the file.

Review Comment:
   Never mind. I fixed this one.



-- 
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

Re: [PR] [Python] Support large pipeline options via file [beam]

Posted by "Mathdee (via GitHub)" <gi...@apache.org>.
Mathdee commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3931183198

   Thanks so much for the review and all your help getting this across the finish line @shunping. So glad we could get it officially merged today. Have a great rest of your week!


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #37379:
URL: https://github.com/apache/beam/pull/37379#discussion_r2737621966


##########
sdks/python/container/boot.go:
##########
@@ -220,7 +220,18 @@ func launchSDKProcess() error {
 
 	// (3) Invoke python
 
-	os.Setenv("PIPELINE_OPTIONS", options)
+	//Commented out -->

Review Comment:
   Clean up comments



-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3860137606

   Reminder, please take a look at this pr: @claudevdm 


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3877310594

   Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment `assign to next reviewer`:
   
   R: @damccorm for label python.
   
   Available commands:
   - `stop reviewer notifications` - opt out of the automated review tooling
   - `remind me after tests pass` - tag the comment author after tests pass
   - `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the 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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #37379:
URL: https://github.com/apache/beam/pull/37379#discussion_r2737558986


##########
sdks/python/container/boot.go:
##########
@@ -503,3 +514,21 @@ func logSubmissionEnvDependencies(ctx context.Context, bufLogger *tools.Buffered
 	bufLogger.Printf(ctx, "%s", string(content))
 	return nil
 }
+
+// MakePipelineOptionsFileAndEnvVar writes the pipeline options to a file.
+// Assumes the options string is JSON formatted.
+//
+// Stores the file name in question in PIPELINE_OPTIONS_FILE for access by the SDK.
+func MakePipelineOptionsFileAndEnvVar(options string) (string, error) {

Review Comment:
   Any reason to redefine this function? I see we already have that in https://github.com/apache/beam/blob/928ad7e2aab52ce5bd36895bae1d17bd9d8d5124/sdks/go/container/tools/pipeline_options.go#L28



-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3821080861

   Let's give it another try. :)


-- 
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

Re: [PR] [Python] Support large pipeline options via file [beam]

Posted by "Mathdee (via GitHub)" <gi...@apache.org>.
Mathdee commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3929864214

   I fixed the notes to include the issue link instead of the PR link, just waiting on the tests to be completed now.


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "Mathdee (via GitHub)" <gi...@apache.org>.
Mathdee commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3781740358

   All linting and integration checks are passing. Ready for review


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3878079389

   R: @shunping 
   
   Assigning to Shunping since you started a review - thanks


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "Mathdee (via GitHub)" <gi...@apache.org>.
Mathdee commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3922904119

   Hey @shunping. Just checking in on this one. Let me know if there's anything else you need from my end.


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3923174326

   > Hey @shunping. Just checking in on this one. Let me know if there's anything else you need from my end.
   
   Sorry for the delay. I would want to build a custom container based on this and test before approving it. Will try to do that tomorrow.


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #37379:
URL: https://github.com/apache/beam/pull/37379#discussion_r2824475498


##########
sdks/python/apache_beam/runners/worker/sdk_worker_main.py:
##########
@@ -93,8 +93,22 @@ def create_harness(environment, dry_run=False):
   else:
     fn_log_handler = None
 
-  pipeline_options_dict = _load_pipeline_options(
-      environment.get('PIPELINE_OPTIONS'))
+  options_json = environment.get('PIPELINE_OPTIONS')
+
+  # We check if options are stored in the file.
+  if 'PIPELINE_OPTIONS_FILE' in environment:
+    options_file = environment['PIPELINE_OPTIONS_FILE']
+    try:
+      with open(options_file, 'r') as f:
+        options_json = f.read()
+        _LOGGER.info('Load pipeline options from file: %s', options_file)
+    except:
+      _LOGGER.error(
+          'Failed to load pipeline options from file: %s', options_file)
+      raise

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   Using a bare `except:` is generally discouraged as it can catch system-exiting exceptions like `SystemExit` or `KeyboardInterrupt`. It's better to catch a more specific exception, such as `Exception`. Additionally, including the exception details in the log message by using `exc_info=True` would be helpful for debugging.
   
   ```suggestion
       except Exception:
         _LOGGER.error(
             'Failed to load pipeline options from file: %s',
             options_file,
             exc_info=True)
         raise
   ```



-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #37379:
URL: https://github.com/apache/beam/pull/37379#discussion_r2737921919


##########
sdks/python/container/boot.go:
##########
@@ -220,7 +220,18 @@ func launchSDKProcess() error {
 
 	// (3) Invoke python
 
-	os.Setenv("PIPELINE_OPTIONS", options)
+	//Commented out -->
+	//os.Setenv("PIPELINE_OPTIONS", options)
+
+	//To prevent crashes if options > ARG_MAX -->
+	// --> Write the large JSON content into a file on disk
+	// --> environment only receives the short filename
+	if optionsFile, err := MakePipelineOptionsFileAndEnvVar(options); err != nil {
+		logger.Fatalf(ctx, "Failed to create a pipeline options file: %v", err)

Review Comment:
   For consistency, could we use the same error message as Java and GO Sdks?
   
   https://github.com/apache/beam/blob/928ad7e2aab52ce5bd36895bae1d17bd9d8d5124/sdks/go/container/boot.go#L198-L199
   
   https://github.com/apache/beam/blob/928ad7e2aab52ce5bd36895bae1d17bd9d8d5124/sdks/java/container/boot.go#L126-L127



-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #37379:
URL: https://github.com/apache/beam/pull/37379#discussion_r2737558986


##########
sdks/python/container/boot.go:
##########
@@ -503,3 +514,21 @@ func logSubmissionEnvDependencies(ctx context.Context, bufLogger *tools.Buffered
 	bufLogger.Printf(ctx, "%s", string(content))
 	return nil
 }
+
+// MakePipelineOptionsFileAndEnvVar writes the pipeline options to a file.
+// Assumes the options string is JSON formatted.
+//
+// Stores the file name in question in PIPELINE_OPTIONS_FILE for access by the SDK.
+func MakePipelineOptionsFileAndEnvVar(options string) (string, error) {

Review Comment:
   Any reason to redefine this function? I see we already have that at
   https://github.com/apache/beam/blob/928ad7e2aab52ce5bd36895bae1d17bd9d8d5124/sdks/go/container/tools/pipeline_options.go#L28, and we already imported the Go package in this `boot.go` at 
   https://github.com/apache/beam/blob/928ad7e2aab52ce5bd36895bae1d17bd9d8d5124/sdks/python/container/boot.go#L38



-- 
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

Re: [PR] [Python] Support large pipeline options via file [beam]

Posted by "Mathdee (via GitHub)" <gi...@apache.org>.
Mathdee commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3929622102

   @shunping Awesome, thanks for taking the time to test this and for adding the log level fallback.
   I just updated the CHANGES.md with the notes exactly as requested. Let me know if it looks good!


-- 
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

Re: [PR] [Python] Support large pipeline options via file [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3929816916

   Loosk like [PreCommit Whitespace](https://github.com/apache/beam/actions/runs/22197740263/job/64202363602?pr=37379) failed.
   
   Could you fix the notes to include the issue link (not the PR link)?
   
   The error message is shown below: 
   ```
   Execution failed for task ':validateChanges'.
   > CHANGES.md validation failed with the following errors:
     Line 71: Missing or malformed issue link. Each entry should end with ([#X](https://github.com/apache/beam/issues/X)): * Added support for large pipeline options via a file (Python) ([#37379](https://github.com/apache/beam/pull/37379)).
     Line 75: Missing or malformed issue link. Each entry should end with ([#X](https://github.com/apache/beam/issues/X)): * The Python SDK container's `boot.go` now passes pipeline options through a file instead of the `PIPELINE_OPTIONS` environment variable. If a user pairs a new Python SDK container with an older SDK version (which does not support the file-based approach), the pipeline options will not be recognized and the pipeline will fail. Users must ensure their SDK and container versions are synchronized ([#37379](https://github.com/apache/beam/pull/37379)).
   ```


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3878091380

   Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, 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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3923228220

   Could you merge to the latest master and push again? @Mathdee 


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3923223443

   /gemini review


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #37379:
URL: https://github.com/apache/beam/pull/37379#discussion_r2737599086


##########
sdks/python/container/boot.go:
##########
@@ -220,7 +220,18 @@ func launchSDKProcess() error {
 
 	// (3) Invoke python
 
-	os.Setenv("PIPELINE_OPTIONS", options)
+	//Commented out -->
+	//os.Setenv("PIPELINE_OPTIONS", options)
+
+	//To prevent crashes if options > ARG_MAX -->
+	// --> Write the large JSON content into a file on disk
+	// --> environment only receives the short filename
+	if optionsFile, err := MakePipelineOptionsFileAndEnvVar(options); err != nil {
+		logger.Fatalf(ctx, "Failed to create a pipeline options file: %v", err)
+	} else {
+		os.Setenv("PIPELINE_OPTIONS_FILE", optionsFile)

Review Comment:
   In either the original `MakePipelineOptionsFileAndEnvVar` or your own copy (which seems to be unnecessary), the environment variable is already set. I think we don't need to do that again 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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #37379:
URL: https://github.com/apache/beam/pull/37379#discussion_r2737632399


##########
sdks/python/container/boot.go:
##########
@@ -220,7 +220,18 @@ func launchSDKProcess() error {
 
 	// (3) Invoke python
 
-	os.Setenv("PIPELINE_OPTIONS", options)
+	//Commented out -->
+	//os.Setenv("PIPELINE_OPTIONS", options)
+
+	//To prevent crashes if options > ARG_MAX -->

Review Comment:
   Could you follow the comment style of this file to polish your comment here?
   
   Something like:
   ```
   // Write the JSON string of pipeline options into a file to prevent `argument list too long` 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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "Mathdee (via GitHub)" <gi...@apache.org>.
Mathdee commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3821193568

   All checks passed! Ready for final review/merge when you are. Thanks again @shunping.


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3927839431

   The failed test workflow [PreCommit YAML Xlang Direct](https://github.com/apache/beam/actions/runs/22162947962/job/64088881187?pr=37379) was unrelated to the test change 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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3928943099

   I’ve verified the changes with a new container and a test pipeline on Dataflow. The pipeline options were successfully loaded via the file and the job completed as expected.
   
   During testing, I found that the INFO log message for loading options from the file was being suppressed. This occurs because the default log level is WARNING until the pipeline options are fully parsed. 
   
   Therefor, I’ve added a fallback to INFO at the start of `create_harness` to ensure these early startup events are captured.


-- 
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

Re: [PR] [Python] Support large pipeline options via file [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping merged PR #37379:
URL: https://github.com/apache/beam/pull/37379


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #37379:
URL: https://github.com/apache/beam/pull/37379#discussion_r2824442759


##########
sdks/python/apache_beam/runners/worker/sdk_worker_main.py:
##########
@@ -93,8 +93,22 @@ def create_harness(environment, dry_run=False):
   else:
     fn_log_handler = None
 
-  pipeline_options_dict = _load_pipeline_options(
-      environment.get('PIPELINE_OPTIONS'))
+  options_json = environment.get('PIPELINE_OPTIONS')
+
+  #We check if options are stored in the file.

Review Comment:
   nit: add space after #



-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "Mathdee (via GitHub)" <gi...@apache.org>.
Mathdee commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3821071318

   @shunping   I saw the re-run failed again, so I dug into the logs. It looks like it's hitting the exact same timeout (`DEADLINE_EXCEEDED`) in `test_builtin_combines`.
   Since that test is totally unrelated to the pipeline options logic (and the other 7,610 tests passed), this seems like a flake in the test environment. 
   Is there a way to merge despite this, or should we try triggering it one more time?
   
   


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3781868583

   Assigning reviewers:
   
   R: @claudevdm for label python.
   
   
   Note: If you would like to opt out of this review, comment `assign to next reviewer`.
   
   Available commands:
   - `stop reviewer notifications` - opt out of the automated review tooling
   - `remind me after tests pass` - tag the comment author after tests pass
   - `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
   
   The PR bot will only process comments in the main thread (not review comments).


-- 
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

Re: [PR] [Python] Support large pipeline options via file (Fixes #37370) [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #37379:
URL: https://github.com/apache/beam/pull/37379#issuecomment-3793380746

   Ack! Will do


-- 
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