[BEAM-4032]Support staging binary distributions of dependency packages #16633

AnandInguva · 2022-01-27T18:36:54Z

This PR changes how we stage packages mentioned in the --requirements_file. Prior to this, the default behavior was to download the source distributions of the packages specified in the --requirements_file during staging. The current behavior would be to download binary distributions(wheels) for packages specified in the --requirements_file without their sub-dependencies. To follow the prior behavior, set --requirements_cache_only_sources. Staging of the packages/pipeline dependencies can be skipped by setting --requirements_cache=skip during job submission.

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) and mention them in a comment (R: @username).
Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
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.

`ValidatesRunner` compliance status (on master branch)

Lang	ULR	Twister2
Go	---	---
Java
Python	---	---
XLang		---

Examples testing status on various runners

Lang	ULR	Dataflow	Flink	Samza	Spark	Twister2
Go	---	---	---	---	---	---	---
Java	---		---	---	---	---	---
Python	---	---	---	---	---	---	---
XLang	---	---	---	---	---	---	---

Post-Commit SDK/Transform Integration Tests Status (on master branch)

Go	Java	Python

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website	Whitespace	Typescript
Non-portable
Portable	---			---	---	---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

GitHub Actions Tests Status (on master branch)

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

codecov · 2022-01-27T20:09:42Z

Codecov Report

Merging #16633 (9b3a04a) into master (fee1528) will increase coverage by 8.94%.
The diff coverage is 53.57%.

@@            Coverage Diff             @@
##           master   #16633      +/-   ##
==========================================
+ Coverage   74.64%   83.59%   +8.94%     
==========================================
  Files         655      452     -203     
  Lines       82325    62274   -20051     
==========================================
- Hits        61454    52058    -9396     
+ Misses      19871    10216    -9655     
+ Partials     1000        0    -1000

Impacted Files	Coverage Δ
...s/python/apache_beam/runners/portability/stager.py	`85.54% <51.85%> (-2.82%)`	⬇️
...dks/python/apache_beam/options/pipeline_options.py	`95.28% <100.00%> (+0.01%)`	⬆️
.../python/apache_beam/transforms/periodicsequence.py	`96.72% <0.00%> (-1.64%)`	⬇️
sdks/python/apache_beam/internal/metrics/metric.py	`90.00% <0.00%> (-1.00%)`	⬇️
...hon/apache_beam/runners/worker/bundle_processor.py	`93.51% <0.00%> (-0.25%)`	⬇️
sdks/go/pkg/beam/core/runtime/exec/fullvalue.go
sdks/go/pkg/beam/core/graph/node.go
sdks/go/pkg/beam/core/runtime/genx/genx.go
sdks/go/pkg/beam/core/graph/coder/map.go
sdks/go/pkg/beam/core/metrics/dumper.go
... and 199 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fee1528...9b3a04a. Read the comment docs.

AnandInguva · 2022-01-28T00:15:14Z

R: @tvalentyn

AnandInguva · 2022-01-28T05:23:29Z

Run Python PreCommit

AnandInguva · 2022-01-28T21:59:02Z

cc @robertwb

AnandInguva · 2022-01-28T22:00:19Z

sdks/python/container/piputil.go

@@ -51,7 +51,7 @@ func pipInstallRequirements(files []string, dir, name string) error {
 			// used without following their dependencies.
 			args := []string{"install", "-r", filepath.Join(dir, name), "--disable-pip-version-check", "--no-index", "--no-deps", "--find-links", dir}
 			if err := execx.Execute(pip, args...); err != nil {
-				return err
+		        fmt.Println("Requirements cache " + dir + " is empty. Downloading packages from PyPI")


Printing this in case if the first Pip call fails(when user skips populating requirements cache) to provide more context if user looks into the logs

sdks/python/apache_beam/options/pipeline_options.py

tvalentyn · 2022-01-28T05:56:02Z

sdks/python/apache_beam/options/pipeline_options.py

-            'the requirements file using the --requirements_file option.'))
+            'the requirements file using the --requirements_file option.'
+            'If you want to skip populating requirements cache, please '
+            'specify --requirements_cache skip. This would install all'


I would remove the last sentence.

tvalentyn · 2022-01-28T05:56:29Z

sdks/python/apache_beam/options/pipeline_options.py

+        '--requirements_cache_only_sources',
+        action='store_true',
+        help=(
+            'Enable this flag to populate requirements cache with Source'


Suggested change

'Enable this flag to populate requirements cache with Source'

'Enable this flag to populate requirements cache only with Source'

tvalentyn · 2022-01-28T06:11:31Z

sdks/python/apache_beam/options/pipeline_options.py

+            'Enable this flag to populate requirements cache with Source'
+            'distributions(sdists) of the dependencies mentioned in the '
+            '--requirements_file'
+            'Note: This step would slow down the worker startup time'


Note (BEAM-XXXX): This flag may significantly slow down the pipeline submission. It is added to preserve the requirements cache behavior prior to 2.37.0 and will likely be removed in future releases.

Added BEAM-4032, since it has the discussion thread, and comments on this issue

tvalentyn · 2022-01-28T06:16:08Z

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

-          if setup_options.requirements_cache is None else
+          os.path.join(tempfile.gettempdir(), 'dataflow-requirements-cache') if
+          (setup_options.requirements_cache is None) and
+          (setup_options.requirements_cache != SKIP_REQUIREMENTS_CACHE) else


and... part seems not necessary

tvalentyn · 2022-01-28T22:11:44Z

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

+    # TODO(anandinguva): When https://github.com/pypa/pip/issues/10760 is
+    # addressed download wheel based on glib version in Beam's Python Base image
+    pip_version = pkg_resources.get_distribution('pip').version
+    if float(pip_version[0:4]) >= 19.3:


This is not reliable, for example it would fail on pip < 10; let's use an off-the-shelf helper that knows how to parse
and compare versions, see: pkg_resources.parse_version or packaging.version.parse

tvalentyn · 2022-01-29T00:42:42Z

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

+          os.path.join(temp_dir, 'nothing.tar.gz'), 'Fake tarball')
+    else:
+      self.create_temp_file(
+          os.path.join(temp_dir, 'nothing.tar.gz'), 'Fake tarball')


appears in both branches

If we download whls and if a whl is not present on PyPI, we download sdist. That is the reason why it is present in both branches.

I meant that you can add nothing.tar.gz outside the if since this code is in both branches. But feel free to keep as is if you think it's more clear this way.

tvalentyn · 2022-01-29T00:52:21Z

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

@@ -342,6 +343,47 @@ def test_with_requirements_file_and_cache(self):
    self.assertTrue(os.path.isfile(os.path.join(staging_dir, 'abc.txt')))
    self.assertTrue(os.path.isfile(os.path.join(staging_dir, 'def.txt')))

+  def test_with_requirements_file_skipping_cache(self):


it's possible to rename the test method to show what's being tested and expected outcome, this makes test cases easier to follow. for example:

test_requirements_cache_not_populated_when_cache_disabled

tvalentyn · 2022-01-29T00:55:41Z

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

+
+    resources = self.stager.create_and_stage_job_resources(
+        options,
+        populate_requirements_cache=self.populate_requirements_cache,


alternatively, you could pass a mock method and verify that it was never called.

tvalentyn · 2022-01-29T01:20:02Z

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

@@ -723,6 +765,65 @@ def test_remove_dependency_from_requirements(self):
    self.assertEqual(['apache_beam\n', 'avro-python3\n', 'numpy\n'],
                     sorted(lines))

+  def _create_file(


Suggested change

def _create_file(

def _populate_requitements_cache_fake(

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

AnandInguva · 2022-02-03T17:32:16Z

PTAL: @tvalentyn

AnandInguva · 2022-02-04T16:13:49Z

Run Python 3.8 PostCommit

AnandInguva · 2022-02-07T14:54:48Z

Run Python 3.8 PostCommit

tvalentyn · 2022-02-07T22:39:15Z

sdks/python/apache_beam/examples/complete/juliaset/requirements.txt

+#  See the License for the specific language governing permissions and
+# limitations under the License.
+###############################################################################    
+numpy


This example already requires numpy in

beam/sdks/python/apache_beam/examples/complete/juliaset/setup.py

Line 111 in 1693483

'numpy',

Given that you are modifying the example, users may be confused why numpy needs to be specified twice: in a requirements.txt file and in setup.py.

Let's try to use a different pipeline for this test. You could create an example where a requirements file is used, or just a test pipeline that does something trivial, like checking whether a particular library is installed and doesn't do any data processing.

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

…nts.txt

This reverts commit d95f2d0.

AnandInguva · 2022-02-09T16:22:26Z

PTAL @tvalentyn. Added a trivial Integration test to check if the specified packages in the requirements.txt are installed on the worker container.

Assert passes if package is installed in the container.
Assert fails if the package is not installed in the container(happens when the package is not specified in the requirements file.)

I will resolve conflicts once this code is good to go

sdks/python/apache_beam/examples/complete/check_requirements_file_packages.py

sdks/python/test-suites/portable/common.gradle

sdks/python/apache_beam/examples/complete/check_requirements_file_packages.py

Creates requirements text for the integration test during runtime

AnandInguva · 2022-02-09T20:57:57Z

Run Python 3.8 PostCommit

tvalentyn · 2022-02-09T21:09:13Z

see also lint failures: https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Commit/12436/

AnandInguva · 2022-02-09T21:14:24Z

https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Commit/12436/

Yes, I already created a commit on that locally. I used to have a lint checker installed but later I disabled it for some reason. I have to enable it now(it would make my life a lot easier)

tvalentyn · 2022-02-09T21:15:17Z

Portable precommit fails due to a side effect from my testing on #16008. Sorry about that.

tvalentyn · 2022-02-10T20:56:15Z

Run Portable_Python PreCommit

AnandInguva · 2022-02-10T21:24:39Z

PTAL @tvalentyn. Thanks

CHANGES.md

AnandInguva · 2022-02-10T22:53:34Z

Run Python 3.8 PostCommit

AnandInguva · 2022-02-11T02:35:44Z

PTAL @tvalentyn

tvalentyn · 2022-02-11T17:46:25Z

Merging, thanks! Please update the PR description.

AnandInguva force-pushed the stage-wheels branch from 7b374c6 to 2965473 Compare January 27, 2022 18:41

AnandInguva force-pushed the stage-wheels branch from 2965473 to 7c42c08 Compare January 27, 2022 20:17

populate requirements cache wheels for linux_x86_64

e310036

AnandInguva force-pushed the stage-wheels branch from 7c42c08 to e310036 Compare January 27, 2022 20:19

Don't return error and let the second pip install through

29beb77

AnandInguva commented Jan 28, 2022

View reviewed changes

tvalentyn reviewed Jan 29, 2022

View reviewed changes

AnandInguva and others added 4 commits January 28, 2022 23:34

Update sdks/python/apache_beam/options/pipeline_options.py

cd4cda2

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

Refactoring tests and stager.py

d0c52f6

Merge remote-tracking branch 'origin/stage-wheels' into stage-wheels

7a9a6f0

Integration test with --requirements_file

ec99a96

github-actions bot added docker examples python labels Feb 3, 2022

Change message when the first pip install fails in boot.go

b0cb013

AnandInguva mentioned this pull request Feb 4, 2022

[BEAM-4032]Download whl for a known target platform on the container image else download sources #16448

Closed

4 tasks

AnandInguva added 2 commits February 4, 2022 10:58

Refactor code

3fa9480

Add license

0dcd5fc

tvalentyn reviewed Feb 7, 2022

View reviewed changes

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

AnandInguva added 3 commits February 8, 2022 23:39

Rename: remove extra underscore in method name

e9f1124

IT to check if a package installed on worker container from requireme…

0eb0674

…nts.txt

resolve conflicts

d95f2d0

Revert "resolve conflicts"

6fe6d4b

This reverts commit d95f2d0.

Fixup lint

1d5f8b5

tvalentyn requested changes Feb 9, 2022

View reviewed changes

Refactor integration test.

af9b886

Creates requirements text for the integration test during runtime

fixup: lint

bccc7b8

AnandInguva added 4 commits February 9, 2022 21:27

Merge remote-tracking branch 'upstream/master' into stage-wheels

3df1666

Merge remote-tracking branch 'upstream/master' into stage-wheels

b9a2751

Add staging wheels feature to the CHANGES.md

ebaaed0

fixup lint

9f5f0ee

tvalentyn reviewed Feb 10, 2022

View reviewed changes

CHANGES.md Outdated Show resolved Hide resolved

AnandInguva added 2 commits February 10, 2022 17:49

Refactor integration test

aa7af77

Add description to CHANGES.md

4d36d80

github-actions bot removed the examples label Feb 10, 2022

Fixup lint

9b3a04a

tvalentyn approved these changes Feb 11, 2022

View reviewed changes

tvalentyn merged commit 7975127 into apache:master Feb 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BEAM-4032]Support staging binary distributions of dependency packages #16633

[BEAM-4032]Support staging binary distributions of dependency packages #16633

AnandInguva commented Jan 27, 2022 •

edited

Loading

codecov bot commented Jan 27, 2022 •

edited

Loading

AnandInguva commented Jan 28, 2022

AnandInguva commented Jan 28, 2022

AnandInguva commented Jan 28, 2022

AnandInguva Jan 28, 2022

tvalentyn Jan 28, 2022

tvalentyn Jan 28, 2022

tvalentyn Jan 28, 2022

AnandInguva Feb 4, 2022

tvalentyn Jan 28, 2022

tvalentyn Jan 28, 2022

tvalentyn Jan 29, 2022

AnandInguva Jan 30, 2022

tvalentyn Feb 1, 2022

AnandInguva Feb 4, 2022

tvalentyn Jan 29, 2022

AnandInguva Feb 4, 2022

tvalentyn Jan 29, 2022

AnandInguva Feb 4, 2022

tvalentyn Jan 29, 2022

AnandInguva commented Feb 3, 2022

AnandInguva commented Feb 4, 2022

AnandInguva commented Feb 7, 2022

tvalentyn Feb 7, 2022

AnandInguva commented Feb 9, 2022

AnandInguva commented Feb 9, 2022

tvalentyn commented Feb 9, 2022

AnandInguva commented Feb 9, 2022

tvalentyn commented Feb 9, 2022

tvalentyn commented Feb 10, 2022

AnandInguva commented Feb 10, 2022

AnandInguva commented Feb 10, 2022

AnandInguva commented Feb 11, 2022

tvalentyn commented Feb 11, 2022

	'Enable this flag to populate requirements cache with Source'
	'Enable this flag to populate requirements cache only with Source'

[BEAM-4032]Support staging binary distributions of dependency packages #16633

[BEAM-4032]Support staging binary distributions of dependency packages #16633

Conversation

AnandInguva commented Jan 27, 2022 • edited Loading

ValidatesRunner compliance status (on master branch)

Examples testing status on various runners

Post-Commit SDK/Transform Integration Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

GitHub Actions Tests Status (on master branch)

codecov bot commented Jan 27, 2022 • edited Loading

Codecov Report

AnandInguva commented Jan 28, 2022

AnandInguva commented Jan 28, 2022

AnandInguva commented Jan 28, 2022

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

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

AnandInguva commented Feb 3, 2022

AnandInguva commented Feb 4, 2022

AnandInguva commented Feb 7, 2022

Choose a reason for hiding this comment

AnandInguva commented Feb 9, 2022

AnandInguva commented Feb 9, 2022

tvalentyn commented Feb 9, 2022

AnandInguva commented Feb 9, 2022

tvalentyn commented Feb 9, 2022

tvalentyn commented Feb 10, 2022

AnandInguva commented Feb 10, 2022

AnandInguva commented Feb 10, 2022

AnandInguva commented Feb 11, 2022

tvalentyn commented Feb 11, 2022

AnandInguva commented Jan 27, 2022 •

edited

Loading

`ValidatesRunner` compliance status (on master branch)

codecov bot commented Jan 27, 2022 •

edited

Loading