[Python]Don't stage beam SDK in Stager #27436

AnandInguva · 2023-07-10T19:36:38Z

Support for DataflowRunner v1 will be deprecated from the version 2.50.0. As part of the effort, we can remove staging the beam SDK that is downloaded from pypi by default.

This PR changes the default value of sdk_container to container

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.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

codecov · 2023-07-10T20:28:55Z

Codecov Report

Merging #27436 (708f058) into master (488279c) will decrease coverage by 0.01%.
Report is 2 commits behind head on master.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##           master   #27436      +/-   ##
==========================================
- Coverage   70.88%   70.87%   -0.01%     
==========================================
  Files         861      861              
  Lines      105040   104999      -41     
==========================================
- Hits        74454    74415      -39     
+ Misses      29027    29025       -2     
  Partials     1559     1559

Flag	Coverage Δ
python	`79.83% <87.50%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
...dks/python/apache_beam/options/pipeline_options.py	`94.20% <ø> (ø)`
...s/python/apache_beam/runners/portability/stager.py	`84.39% <87.50%> (-2.65%)`	⬇️

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

AnandInguva · 2023-07-10T21:04:27Z

Run Python 3.10 PostCommit

AnandInguva · 2023-07-10T21:04:37Z

Run Python 3.11 PostCommit

github-actions · 2023-07-10T21:34:42Z

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @riteshghorse 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)

The PR bot will only process comments in the main thread (not review comments).

riteshghorse

LGTM with minor nits. Thanks!

riteshghorse · 2023-07-12T14:56:40Z

sdks/python/apache_beam/runners/portability/stager.py

+                'The --sdk_location option was used with an unsupported '
+                'type of location: %s' % sdk_location)
+
+        elif sdk_location == 'default' or sdk_location == 'container':


can we still keep the comment here

# Use the SDK that's built into the container, rather than re-staging it.

Sg. I will re-add the comment along with some more context

elif sdk_location == 'default': # Use default location for a runner. pass elif sdk_location == 'container': # Used in the past to indicate that SDK should be used from container image instead of being staged. # Equivalent to 'default' now, leaving for backwards compatibility reasons. pass

sdks/python/apache_beam/runners/portability/stager.py

AnandInguva · 2023-07-12T17:10:00Z

R: @tvalentyn

github-actions · 2023-07-12T17:11:16Z

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

tvalentyn · 2023-07-12T21:32:31Z

let's announce a breaking change in changes.md
let's add a check in boot.go that would fail with a meaningful error if a (custom) container doesn't include beam.

riteshghorse

changes to go files looks good

sdks/python/container/boot.go

AnandInguva · 2023-07-25T15:12:03Z

Run Python Dataflow ValidatesContainer

AnandInguva · 2023-07-25T15:38:56Z

Run Python Dataflow ValidatesContainer

sdks/python/container/piputil.go

AnandInguva · 2023-07-26T08:09:25Z

Run Python RC Dataflow ValidatesContainer

CHANGES.md

sdks/python/apache_beam/options/pipeline_options.py

sdks/python/container/piputil.go

tvalentyn · 2023-07-26T16:55:07Z

sdks/python/apache_beam/options/pipeline_options.py

-            'Override the default location from where the Beam SDK is '
-            'downloaded. It can be a URL, a GCS path, or a local path to an '
+            'Override the default location of the beam SDK. '
+            'It can be a URL, a GCS path, or a local path to an '
            'SDK tarball. Workflow submissions will download or copy an SDK '


Use this flag when running pipelines with an unreleased or manually patched version of Beam SDK.

tvalentyn · 2023-07-26T17:06:11Z

sdks/python/apache_beam/options/pipeline_options.py

-            'tarball from here. If set to the string "default", a standard '
-            'SDK location is used. If empty, no SDK is copied.'))
+            'tarball from here. If set to the string "default", '
+            'the beam SDK in the default container will be used.'))


If set to the string "default", runners will use the SDK provided in the default environment.

tvalentyn · 2023-07-26T17:16:44Z

sdks/python/apache_beam/runners/portability/stager.py

+                'The --sdk_location option was used with an unsupported '
+                'type of location: %s' % sdk_location)
+
+        elif sdk_location == 'default' or sdk_location == 'container':


elif sdk_location == 'default': # Use default location for a runner. pass elif sdk_location == 'container': # Used in the past to indicate that SDK should be used from container image instead of being staged. # Equivalent to 'default' now, leaving for backwards compatibility reasons. pass

tvalentyn · 2023-07-26T17:19:21Z

sdks/python/container/boot.go

+	pkgName := "apache-beam"
+	isSdkInstalled, err := isPackageInstalled(pkgName)
+	if err != nil {
+		return fmt.Errorf("failed to check if Apache Beam %s is installed: %v", pkgName, err)


this shouldn't be a hard error, we can attempt to continue.

I removed this entirely.

sdks/python/container/boot.go

tvalentyn · 2023-07-26T18:07:22Z

sdks/python/container/boot.go

+	}
+
+	if !isSdkInstalled {
+		log.Printf("Apache Beam is not installed on the custom container. Please make sure Apache Beam is installed in the custom container. Look at the docummentation https://beam.apache.org/documentation/runtime/environments/ on custom containers for more details.")


wording suggestion: "Apache Beam is not installed in the runtime environment. If you use a custom container image, you must install apache-beam package in the custom image using same version of Beam as in the pipeline submission environment. For more information, see: the https://beam.apache.org/documentation/runtime/environments/ page."

Changed wording

Co-authored-by: tvalentyn <tvalentyn@users.noreply.github.com>

sdks/python/apache_beam/runners/portability/stager.py

sdks/python/container/boot.go

sdks/python/container/piputil.go

tvalentyn · 2023-07-28T23:46:25Z

Run Python_Runners PreCommit

tvalentyn · 2023-07-28T23:48:53Z

Thanks, changes look good to me. have you tested them in positive and negative case? Negative case requries building a custom docker image similar to https://cloud.google.com/dataflow/docs/guides/using-custom-containers#use_a_custom_base_image_or_multi-stage_builds but w/o Beam.

If you are already OOO i can TAL next week, np.

AnandInguva · 2023-08-02T06:17:27Z

Thanks, changes look good to me. have you tested them in positive and negative case? Negative case requries building a custom docker image similar to https://cloud.google.com/dataflow/docs/guides/using-custom-containers#use_a_custom_base_image_or_multi-stage_builds but w/o Beam.

If you are already OOO i can TAL next week, np.

This job uses the container gcr.io/google.com/clouddfe/anandinguva:2.50.0.dev with beam installed in it. I can the see the message in the logs but at INFO level.

https://pantheon.corp.google.com/dataflow/jobs/us-central1/2023-08-01_23_06_17-11941549679963536368;mainTab=JOB_GRAPH;bottomTab=WORKER_LOGS;logsSeverity=INFO;graphView=0?project=google.com:clouddfe&pageState=(%22dfTime%22:(%22l%22:%22dfJobMaxTime%22))

tvalentyn · 2023-08-02T17:52:58Z

hmm.. that job has errors like:

DEBUG 2023-08-02T07:02:33.534971077Z Executing Python (worker sdk-0-0): python -m apache_beam.runners.worker.sdk_worker_main
INFO 2023-08-02T07:02:33.596045792Z /opt/apache/beam-venv/beam-venv-worker-sdk-0-0/bin/python: Error while finding module specification for 'apache_beam.runners.worker.sdk_worker_main' (ModuleNotFoundError: No module named 'apache_beam')
INFO 2023-08-02T07:02:33.596088772Z /opt/apache/beam-venv/beam-venv-worker-sdk-0-0/bin/python: Error while finding module specification for 'apache_beam.runners.worker.sdk_worker_main' (ModuleNotFoundError: No module named 'apache_beam')

we should expect to see Apache Beam is not installed in the runtime environment..

AnandInguva · 2023-08-04T06:36:10Z

https://pantheon.corp.google.com/dataflow/jobs/us-central1/2023-08-03_23_16_40-2480228236867561239;bottomTab=WORKER_LOGS;logsSeverity=INFO;graphView=0?project=google.com:clouddfe&pageState=(%22dfTime%22:(%22l%22:%22dfJobMaxTime%22))

This is the latest job with the updated boot.go. This is working as expected as we can see the expected errors in the logs.

…unstage_sdk

CHANGES.md

Co-authored-by: tvalentyn <tvalentyn@users.noreply.github.com>

This reverts commit 408d766.

github-actions bot added the python label Jul 10, 2023

AnandInguva force-pushed the unstage_sdk branch 3 times, most recently from 89abd9b to 44b3984 Compare July 10, 2023 20:09

AnandInguva marked this pull request as ready for review July 10, 2023 20:42

AnandInguva changed the title ~~Don't stage beam SDK in Stager~~ [Python]Don't stage beam SDK in Stager Jul 10, 2023

github-actions bot added the Next Action: Reviewers label Jul 10, 2023

riteshghorse approved these changes Jul 12, 2023

View reviewed changes

AnandInguva added 7 commits July 25, 2023 03:12

Remove staging sdk wheel and source

52fd898

Unstage SDK

5227a55

Add note to CHANGES.md

42e39da

Update arg description

30361f6

Add test for sdk_location when default

8a22321

Add a check if Apache Beam is installed on the custom container

74ac6ef

Update CHANGES.md

1413614

AnandInguva force-pushed the unstage_sdk branch from 66ce55d to 1413614 Compare July 25, 2023 08:42

github-actions bot added the docker label Jul 25, 2023

AnandInguva added 2 commits July 25, 2023 05:43

fix print statement

ffe3d75

Fix syntax

ccda6f3

riteshghorse approved these changes Jul 25, 2023

View reviewed changes

riteshghorse reviewed Jul 25, 2023

View reviewed changes

sdks/python/container/boot.go Outdated Show resolved Hide resolved

AnandInguva added 2 commits July 25, 2023 10:05

Add pkgName

d075011

Import package

eb35e2d

AnandInguva commented Jul 25, 2023

View reviewed changes

sdks/python/container/piputil.go Outdated Show resolved Hide resolved

sdks/python/container/piputil.go Outdated Show resolved Hide resolved

sdks/python/container/piputil.go Outdated Show resolved Hide resolved

Apply suggestions from code review

f6c1268

AnandInguva requested a review from tvalentyn July 26, 2023 09:03

tvalentyn requested changes Jul 26, 2023

View reviewed changes

AnandInguva and others added 4 commits July 26, 2023 22:35

use pip show instead of pip list

721ba8c

Merge remote-tracking branch 'origin/unstage_sdk' into unstage_sdk

d79fe64

Apply suggestions from code review

ced822a

Co-authored-by: tvalentyn <tvalentyn@users.noreply.github.com>

Refactor

734e702

tvalentyn reviewed Jul 27, 2023

View reviewed changes

sdks/python/apache_beam/runners/portability/stager.py Outdated Show resolved Hide resolved

sdks/python/container/boot.go Show resolved Hide resolved

sdks/python/container/piputil.go Show resolved Hide resolved

AnandInguva added 2 commits July 27, 2023 23:11

return boolean

d071335

Remove comment

44293ee

AnandInguva requested a review from tvalentyn July 28, 2023 14:08

AnandInguva added 2 commits August 4, 2023 02:37

Merge remote-tracking branch 'apache/nightly-refs/heads/master' into …

bccc4be

…unstage_sdk

Merge remote-tracking branch 'upstream/master' into unstage_sdk

5810eff

AnandInguva commented Aug 4, 2023

View reviewed changes

CHANGES.md Outdated Show resolved Hide resolved

Update CHANGES.md

708f058

tvalentyn merged commit 408d766 into apache:master Aug 4, 2023

BjornPrime pushed a commit to BjornPrime/my-beam that referenced this pull request Aug 7, 2023

[Python]Don't stage beam SDK in Stager (apache#27436)

89b2776

Co-authored-by: tvalentyn <tvalentyn@users.noreply.github.com>

tvalentyn mentioned this pull request Aug 22, 2023

[Feature request] Consider installing the same artifact in Beam Python RC Docker containers as is being published to PyPi #28084

Open

15 tasks

damccorm added a commit that referenced this pull request Aug 22, 2023

Revert "[Python]Don't stage beam SDK in Stager (#27436)"

ace89c9

This reverts commit 408d766.

damccorm mentioned this pull request Aug 22, 2023

Revert "[Python]Don't stage beam SDK in Stager" #28094

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Python]Don't stage beam SDK in Stager #27436

[Python]Don't stage beam SDK in Stager #27436

AnandInguva commented Jul 10, 2023 •

edited

Loading

codecov bot commented Jul 10, 2023 •

edited

Loading

AnandInguva commented Jul 10, 2023

AnandInguva commented Jul 10, 2023

github-actions bot commented Jul 10, 2023

riteshghorse left a comment

riteshghorse Jul 12, 2023

AnandInguva Jul 12, 2023

tvalentyn Jul 26, 2023

AnandInguva commented Jul 12, 2023

github-actions bot commented Jul 12, 2023

tvalentyn commented Jul 12, 2023

riteshghorse left a comment

AnandInguva commented Jul 25, 2023

AnandInguva commented Jul 25, 2023

AnandInguva commented Jul 26, 2023

tvalentyn Jul 26, 2023

AnandInguva Jul 27, 2023

tvalentyn Jul 26, 2023

tvalentyn Jul 26, 2023

tvalentyn Jul 26, 2023

AnandInguva Jul 27, 2023

tvalentyn Jul 26, 2023

AnandInguva Jul 27, 2023

tvalentyn commented Jul 28, 2023

tvalentyn commented Jul 28, 2023

AnandInguva commented Aug 2, 2023 •

edited

Loading

tvalentyn commented Aug 2, 2023 •

edited

Loading

AnandInguva commented Aug 4, 2023

[Python]Don't stage beam SDK in Stager #27436

[Python]Don't stage beam SDK in Stager #27436

Conversation

AnandInguva commented Jul 10, 2023 • edited Loading

GitHub Actions Tests Status (on master branch)

codecov bot commented Jul 10, 2023 • edited Loading

Codecov Report

AnandInguva commented Jul 10, 2023

AnandInguva commented Jul 10, 2023

github-actions bot commented Jul 10, 2023

riteshghorse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AnandInguva commented Jul 12, 2023

github-actions bot commented Jul 12, 2023

tvalentyn commented Jul 12, 2023

riteshghorse left a comment

Choose a reason for hiding this comment

AnandInguva commented Jul 25, 2023

AnandInguva commented Jul 25, 2023

AnandInguva commented Jul 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tvalentyn commented Jul 28, 2023

tvalentyn commented Jul 28, 2023

AnandInguva commented Aug 2, 2023 • edited Loading

tvalentyn commented Aug 2, 2023 • edited Loading

AnandInguva commented Aug 4, 2023

AnandInguva commented Jul 10, 2023 •

edited

Loading

codecov bot commented Jul 10, 2023 •

edited

Loading

AnandInguva commented Aug 2, 2023 •

edited

Loading

tvalentyn commented Aug 2, 2023 •

edited

Loading