[BEAM-9577] Artifact v2 support for uber jars. #11708

robertwb · 2020-05-14T17:15:10Z

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.

Post-Commit Tests Status (on master branch)

Lang	SDK	Apex	Dataflow	Gearpump	Samza
Go		---	---	---	---
Java
Python		---		---	---
XLang	---	---	---	---	---

Pre-Commit Tests Status (on master branch)

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

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

ibzib

Will review the Python part in a bit, but wanted to comment on Java first because I feel I'm missing something.

sdks/java/core/src/main/java/org/apache/beam/sdk/io/ClassLoaderFileSystem.java

ibzib · 2020-05-14T21:22:18Z

sdks/java/core/src/test/java/org/apache/beam/sdk/io/ClassLoaderFileSystemTest.java

+    InputStream inputStream = newInputStream(channel);
+    byte[] magic = new byte[4];
+    inputStream.read(magic);
+    assertArrayEquals(magic, new byte[] {(byte) 0xCA, (byte) 0xFE, (byte) 0xBA, (byte) 0xBE});


How did I never know this? 😆

sdks/java/core/src/main/java/org/apache/beam/sdk/io/ClassLoaderFileSystem.java

robertwb · 2020-05-15T00:30:53Z

Still trying to figure out why the test fails on jenkins but passes locally, but other than that it should be ready to be looked at again.

ibzib · 2020-05-15T18:39:50Z

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

-    self._artifact_staging_service.close()
-    self._artifact_manifest_location = (
-        self._artifact_staging_service.retrieval_token(self._job_id))
+    self._artifact_manifest_location = None


Can't we remove this now?

ibzib · 2020-05-15T18:40:08Z

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

        url='localhost:%d' % port)
    self._artifact_staging_server.start()
    _LOGGER.info('Artifact server started on port %s', port)
+    _LOGGER.error('Artifact server started on port %s', port)


Leftover debugging. Removed.

ibzib · 2020-05-15T18:44:00Z

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

+  def close(self):
+    self._zipfile_handle.close()
+
+  def file_writer(self, path):


Can we get a type annotation and/or comment on the return value here?

ibzib · 2020-05-15T18:50:24Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/ClassLoaderFileSystem.java

+import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableList;
+
+/** A read-only {@link FileSystem} implementation looking up resources using a ClassLoader. */
+public class ClassLoaderFileSystem extends FileSystem<ClassLoaderFileSystem.ClassLoaderResourceId> {


Should we change PortablePipelineJarUtils to use ClassLoaderFileSystem? (Maybe in a follow-up PR) https://github.com/apache/beam/blob/master/runners/java-job-service/src/main/java/org/apache/beam/runners/jobsubmission/PortablePipelineJarUtils.java

Yes, we'll want to do that too (and use the new artifact api).

robertwb

Thanks, PTAL.

robertwb · 2020-05-15T21:39:46Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/ClassLoaderFileSystem.java

+import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableList;
+
+/** A read-only {@link FileSystem} implementation looking up resources using a ClassLoader. */
+public class ClassLoaderFileSystem extends FileSystem<ClassLoaderFileSystem.ClassLoaderResourceId> {


Yes, we'll want to do that too (and use the new artifact api).

robertwb · 2020-05-15T21:46:07Z

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

+  def close(self):
+    self._zipfile_handle.close()
+
+  def file_writer(self, path):


robertwb · 2020-05-15T21:46:38Z

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

        url='localhost:%d' % port)
    self._artifact_staging_server.start()
    _LOGGER.info('Artifact server started on port %s', port)
+    _LOGGER.error('Artifact server started on port %s', port)


Leftover debugging. Removed.

robertwb · 2020-05-15T21:47:13Z

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

-    self._artifact_staging_service.close()
-    self._artifact_manifest_location = (
-        self._artifact_staging_service.retrieval_token(self._job_id))
+    self._artifact_manifest_location = None


ibzib

LGTM

ibzib · 2020-05-15T22:36:42Z

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

  def file_writer(self, path):
+    # type: (str) -> Tuple[BinaryIO, str]
+    """Given a relative path, returns an open handle that can be written to
+    and an reference that can later be used to read this file."""


Suggested change

and an reference that can later be used to read this file."""

and a reference that can later be used to read this file."""

* Support ZetaSQL DATE type as a Beam LogicalType * [BEAM-6733] Add pipeline option to flush bundle data before checkpointing We had a couple of PRs in which we wanted to remove the buffering of bundle output during checkpointing: #7940 #9652. Ultimately, we didn't merge any of those because we weren't sure how the change would affect the checkpoint performance. As a better migration path, this introduces a pipeline option to change the default, buffering bundle output during checkpointing, to finishing the bundle and flushing all data before checkpointing. * Remove all answer placeholder checks as they can be confusing at times for some learners * Update course in Stepik * [BEAM-10018] Fix timestamps in windowing kata In this Kata, the timestamp was calculated from time objects, and converted to a timestamp in the local timezone. Thus, the results of the test depended on the configuration of the local timezone in the running system. The tests were hardcoded with a timezone different to mine, and thus I always failed to pass this Kata. The changes in this commit change the type in Event to be a datetime, the timestamps are set in UTC, and the output in the tests is hardcoded in UTC too. This should ensure that the kata works regardless the timezone configured in the system running the kata. * [BEAM-10018] Kata failing due to failed parsing Parsing the timestamps as strings using fromisoformat was failing, and the Kata failed silently regardless the code written in the boxes. This change sets the same timestamps, with UTC timezone, without parsing strings. * Convert html task description to md for "Hello Beam" and "Core Transforms/Map" * Remove unused import * Add missing dependency * Fix member variable name in Kata documentation * Fix placeholder location * Convert html task description to md for "Core Transforms" remaining lessons * Convert html task description to md for "Common Transforms" lessons * Convert html task description to md for remaining Python Katas lessons * Convert html task description to md for most of Java Katas lessons * Convert html task description to md for Java Katas "Common Transforms" lessons * Convert html task description to md for Java Katas "Core Transforms" lessons * [BEAM-2530] Implement Zeta SQL precommit compile tests and run on java 11 (#11692) [BEAM-2530] Implement Zeta SQL precommit compile tests and run on java 11 * Python3 fix - convert dict.keys() to list before indexing (#11733) * Updates google-apitools and httplib2 (#11726) * [BEAM-9964] Update CHANGES.md (#11743) Co-authored-by: Omar Ismail <omarismail@omarismail-macbookpro.roam.corp.google.com> * [BEAM-9577] Artifact v2 support for uber jars. (#11708) * Adds a "filesystem" for artifacts placed on the classpath (e.g. within the uberjar). * Updates the flink and spark uberjars to use artifact staging v2, leveraging the above filesystem. * Populate all SpannerIO batching parameters in display data. Add all the grouping/batching parameters in SpannerIO populateDisplayData(). * Fix capitalization, clarify descriptions * fix capitalization, clarify description Grouped * Refactor to extract single method for popuplating displayData * Convert html task description to md for "Hello Beam" and "Core Transforms/Map" * Convert html task description to md for "Core Transforms" remaining lessons * Convert html task description to md for "Common Transforms" lessons * Convert html task description to md for remaining Python Katas lessons * Convert html task description to md for most of Java Katas lessons * Convert html task description to md for Java Katas "Common Transforms" lessons * Convert html task description to md for Java Katas "Core Transforms" lessons * Resolve merge conflict * Update Python Katas on Stepik * Update Beam Katas Java on Stepik Co-authored-by: Yueyang Qiu <robinyqiu@gmail.com> Co-authored-by: Maximilian Michels <mxm@apache.org> Co-authored-by: Israel Herraiz <ihr@google.com> Co-authored-by: pawelpasterz <32893017+pawelpasterz@users.noreply.github.com> Co-authored-by: Chamikara Jayalath <chamikara@apache.org> Co-authored-by: tvalentyn <tvalentyn@users.noreply.github.com> Co-authored-by: Pablo <pabloem@users.noreply.github.com> Co-authored-by: omarismail94 <44980219+omarismail94@users.noreply.github.com> Co-authored-by: Omar Ismail <omarismail@omarismail-macbookpro.roam.corp.google.com> Co-authored-by: Andrew Pilloud <apilloud@users.noreply.github.com> Co-authored-by: Robert Bradshaw <robertwb@google.com> Co-authored-by: nielm <nielm@google.com> Co-authored-by: Brian Hulette <hulettbh@gmail.com> Co-authored-by: Brian Hulette <bhulette@google.com>

* Adds a "filesystem" for artifacts placed on the classpath (e.g. within the uberjar). * Updates the flink and spark uberjars to use artifact staging v2, leveraging the above filesystem.

* Support ZetaSQL DATE type as a Beam LogicalType * [BEAM-6733] Add pipeline option to flush bundle data before checkpointing We had a couple of PRs in which we wanted to remove the buffering of bundle output during checkpointing: apache#7940 apache#9652. Ultimately, we didn't merge any of those because we weren't sure how the change would affect the checkpoint performance. As a better migration path, this introduces a pipeline option to change the default, buffering bundle output during checkpointing, to finishing the bundle and flushing all data before checkpointing. * Remove all answer placeholder checks as they can be confusing at times for some learners * Update course in Stepik * [BEAM-10018] Fix timestamps in windowing kata In this Kata, the timestamp was calculated from time objects, and converted to a timestamp in the local timezone. Thus, the results of the test depended on the configuration of the local timezone in the running system. The tests were hardcoded with a timezone different to mine, and thus I always failed to pass this Kata. The changes in this commit change the type in Event to be a datetime, the timestamps are set in UTC, and the output in the tests is hardcoded in UTC too. This should ensure that the kata works regardless the timezone configured in the system running the kata. * [BEAM-10018] Kata failing due to failed parsing Parsing the timestamps as strings using fromisoformat was failing, and the Kata failed silently regardless the code written in the boxes. This change sets the same timestamps, with UTC timezone, without parsing strings. * Convert html task description to md for "Hello Beam" and "Core Transforms/Map" * Remove unused import * Add missing dependency * Fix member variable name in Kata documentation * Fix placeholder location * Convert html task description to md for "Core Transforms" remaining lessons * Convert html task description to md for "Common Transforms" lessons * Convert html task description to md for remaining Python Katas lessons * Convert html task description to md for most of Java Katas lessons * Convert html task description to md for Java Katas "Common Transforms" lessons * Convert html task description to md for Java Katas "Core Transforms" lessons * [BEAM-2530] Implement Zeta SQL precommit compile tests and run on java 11 (apache#11692) [BEAM-2530] Implement Zeta SQL precommit compile tests and run on java 11 * Python3 fix - convert dict.keys() to list before indexing (apache#11733) * Updates google-apitools and httplib2 (apache#11726) * [BEAM-9964] Update CHANGES.md (apache#11743) Co-authored-by: Omar Ismail <omarismail@omarismail-macbookpro.roam.corp.google.com> * [BEAM-9577] Artifact v2 support for uber jars. (apache#11708) * Adds a "filesystem" for artifacts placed on the classpath (e.g. within the uberjar). * Updates the flink and spark uberjars to use artifact staging v2, leveraging the above filesystem. * Populate all SpannerIO batching parameters in display data. Add all the grouping/batching parameters in SpannerIO populateDisplayData(). * Fix capitalization, clarify descriptions * fix capitalization, clarify description Grouped * Refactor to extract single method for popuplating displayData * Convert html task description to md for "Hello Beam" and "Core Transforms/Map" * Convert html task description to md for "Core Transforms" remaining lessons * Convert html task description to md for "Common Transforms" lessons * Convert html task description to md for remaining Python Katas lessons * Convert html task description to md for most of Java Katas lessons * Convert html task description to md for Java Katas "Common Transforms" lessons * Convert html task description to md for Java Katas "Core Transforms" lessons * Resolve merge conflict * Update Python Katas on Stepik * Update Beam Katas Java on Stepik Co-authored-by: Yueyang Qiu <robinyqiu@gmail.com> Co-authored-by: Maximilian Michels <mxm@apache.org> Co-authored-by: Israel Herraiz <ihr@google.com> Co-authored-by: pawelpasterz <32893017+pawelpasterz@users.noreply.github.com> Co-authored-by: Chamikara Jayalath <chamikara@apache.org> Co-authored-by: tvalentyn <tvalentyn@users.noreply.github.com> Co-authored-by: Pablo <pabloem@users.noreply.github.com> Co-authored-by: omarismail94 <44980219+omarismail94@users.noreply.github.com> Co-authored-by: Omar Ismail <omarismail@omarismail-macbookpro.roam.corp.google.com> Co-authored-by: Andrew Pilloud <apilloud@users.noreply.github.com> Co-authored-by: Robert Bradshaw <robertwb@google.com> Co-authored-by: nielm <nielm@google.com> Co-authored-by: Brian Hulette <hulettbh@gmail.com> Co-authored-by: Brian Hulette <bhulette@google.com>

[BEAM-9577] Artifact v2 support for uber jars.

e7371d5

probot-autolabeler bot added java python labels May 14, 2020

robertwb requested a review from ibzib May 14, 2020 17:15

robertwb added 3 commits May 14, 2020 13:00

lint, refactor fix

8a3c140

fix spark like flink

3a75ae8

lint, attempt to use system class loader to make jenkins happy

4edfdaa

ibzib reviewed May 14, 2020

View reviewed changes

robertwb added 2 commits May 14, 2020 15:18

fix build break, more tests

00ecfeb

more debugging

c428a1a

ibzib reviewed May 15, 2020

View reviewed changes

robertwb added 2 commits May 15, 2020 14:36

case insensitive filesystems hide bugs!

582caf2

fixup: reviewer comments

122b159

robertwb commented May 15, 2020

View reviewed changes

ibzib approved these changes May 15, 2020

View reviewed changes

robertwb added 2 commits May 18, 2020 13:20

Fix lint, formatting, test mocks.

2839770

lint import fixes

8a4132f

robertwb merged commit 76fbe45 into apache:master May 18, 2020

	and an reference that can later be used to read this file."""
	and a reference that can later be used to read this file."""

[BEAM-9577] Artifact v2 support for uber jars. #11708

[BEAM-9577] Artifact v2 support for uber jars. #11708

Uh oh!

Conversation

robertwb commented May 14, 2020

Post-Commit Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

Uh oh!

ibzib left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

robertwb commented May 15, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robertwb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ibzib left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants