[BEAM-802] Python templates #2545

aaltay · 2017-04-14T22:36:18Z

Re-reverts the python templates. First two commits are clean reverts of the original commits.

The last commit:

Removes the concept of options id. (Except for a few places marked with TODO. Dataflow workers depend on those paths for now, I will remove them in a follow up PR.)

aaltay · 2017-04-14T22:36:34Z

R: @sb2nov

aaltay · 2017-04-14T22:37:03Z

Run Python PostCommit

asfbot · 2017-04-14T22:48:55Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PostCommit_Python_Verify/1865/
--none--

aaltay · 2017-04-14T22:57:09Z

Run Python PostCommit

asfbot · 2017-04-14T23:19:27Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/9558/

Build result: FAILURE

[...truncated 2.32 MB...] at java.lang.Thread.run(Thread.java:745)Caused by: org.apache.maven.plugin.MojoExecutionException: Command execution failed. at org.codehaus.mojo.exec.ExecMojo.execute(ExecMojo.java:302) at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134) at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208) ... 31 moreCaused by: org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1) at org.apache.commons.exec.DefaultExecutor.executeInternal(DefaultExecutor.java:404) at org.apache.commons.exec.DefaultExecutor.execute(DefaultExecutor.java:166) at org.codehaus.mojo.exec.ExecMojo.executeCommandLine(ExecMojo.java:764) at org.codehaus.mojo.exec.ExecMojo.executeCommandLine(ExecMojo.java:711) at org.codehaus.mojo.exec.ExecMojo.execute(ExecMojo.java:289) ... 33 more2017-04-14T23:18:59.203 [ERROR] 2017-04-14T23:18:59.203 [ERROR] Re-run Maven using the -X switch to enable full debug logging.2017-04-14T23:18:59.203 [ERROR] 2017-04-14T23:18:59.203 [ERROR] For more information about the errors and possible solutions, please read the following articles:2017-04-14T23:18:59.203 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException2017-04-14T23:18:59.203 [ERROR] 2017-04-14T23:18:59.203 [ERROR] After correcting the problems, you can resume the build with the command2017-04-14T23:18:59.203 [ERROR] mvn -rf :beam-sdks-pythonchannel stoppedSetting status of 2b06490 to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/9558/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

asfbot · 2017-04-14T23:40:05Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PostCommit_Python_Verify/1866/
--none--

coveralls · 2017-04-15T00:02:13Z

Coverage increased (+0.003%) to 70.47% when pulling c9f17a6 on aaltay:templ into f30d5b9 on apache:master.

asfbot · 2017-04-15T00:05:13Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/9560/
--none--

sb2nov

It is fine to merge this for now, I'm going to spend sometime on making this runner agnostic next.

sb2nov · 2017-04-17T20:26:42Z

sdks/python/apache_beam/internal/gcp/json_value.py

@@ -104,6 +106,10 @@ def to_json_value(obj, with_type=False):
      raise TypeError('Can not encode {} as a 64-bit integer'.format(obj))
  elif isinstance(obj, float):
    return extra_types.JsonValue(double_value=obj)
+  elif isinstance(obj, ValueProvider):


Update json_values_tests to have this case ?

https://issues.apache.org/jira/browse/BEAM-1998 for @mariapython

sb2nov · 2017-04-17T20:35:41Z

sdks/python/apache_beam/io/filebasedsource.py


+    if (not (isinstance(file_pattern, basestring)


isinstance(file_pattern, (basestring, ValueProvider))

sb2nov · 2017-04-17T21:18:54Z

sdks/python/apache_beam/io/fileio.py

@@ -220,22 +235,33 @@ def close(self, file_handle):
    if file_handle is not None:
      file_handle.close()

+  @check_accessible(['file_path_prefix', 'file_name_suffix'])


@sammcveety random question:
I wanted to learn more about the accessibility check in the value providers:

Pros:

Check is done before any user code is executed so we can minimize any side effects.

Cons:

More change to existing user code and the end user needs to make sure they are using the correct strings in the decorator.

We could just throw a RunTimeException in the get as that minimizes the concepts that the user needs to be aware of when writing the pipeline.

The user can already just ignore the check if they want so I guess it is already sort of optional.

Java SDK does not have a concept similar to check_accesible decorators. The common API across both languages are is_accesible and get, where the second one will throw a RuntimeException if the underlying value is not accesible (i.e. called at pipeline building time).

Using check_accessible is optional. A PTransform author can build a templatable transform without using it. Another pro is, it improves the readability by clearly marking functions that depend on run time values of some value.

sb2nov · 2017-04-17T21:19:20Z

sdks/python/apache_beam/io/fileio_test.py

@@ -93,7 +95,7 @@ class TestFileSink(_TestCaseWithTempDirCleanUp):
  def test_file_sink_writing(self):
    temp_path = os.path.join(self._new_tempdir(), 'filesink')
    sink = MyFileSink(
-        temp_path, file_name_suffix='.foo', coder=coders.ToStringCoder())
+        temp_path, file_name_suffix='.output', coder=coders.ToStringCoder())


why do you need this foo -> output change in this PR

I think output is more descriptive, that is why I made the change.

(kept this as is in the original PR)

sb2nov · 2017-04-17T21:19:45Z

sdks/python/apache_beam/runners/direct/direct_runner.py

@@ -88,6 +89,11 @@ def run(self, pipeline):
                        evaluation_context)
    # Start the executor. This is a non-blocking call, it will start the
    # execution in background threads and return.
+
+    if pipeline.options:
+      # DirectRunner does not support RuntimeValueProviders.


Can you add a BEAM issue to try to address this ?

Maybe this comment needs to be changed. If options are passed in the command line, they will be of type StaticValueProvider and DirectRunner can use them. If they were not passed at the command line (or in another way in PipelineOptions) prior to calling run, there will be no opportunity for the user to tell what the run time values should be. Given that, I do not think DirectRunner can support RuntimeValueProviders.

Why do we need the if here? If None, set_runtime_options will handle it.

sb2nov · 2017-04-17T21:20:04Z

sdks/python/apache_beam/io/fileio.py

-    if not isinstance(file_name_suffix, basestring):
-      raise TypeError('file_name_suffix must be a string; got %r instead' %
-                      file_name_suffix)
+    if not (isinstance(file_path_prefix, basestring)


merge these isinstances

sb2nov · 2017-04-18T00:12:15Z

sdks/python/apache_beam/runners/dataflow/internal/apiclient.py

@@ -233,6 +233,7 @@ def __init__(self, packages, options, environment_version):
      options_dict = {k: v
                      for k, v in sdk_pipeline_options.iteritems()
                      if v is not None}
+      options_dict['_options_id'] = 0  # TODO(altay): Remove.


aaltay · 2017-04-18T18:11:03Z

retest this please

aaltay · 2017-04-18T18:11:16Z

Run Python PostCommit

…Source I/O Transforms"""" This reverts commit 0f5c363.

This reverts commit 4854291.

mariapython · 2017-04-18T20:42:10Z

sdks/python/apache_beam/utils/pipeline_options.py

@@ -119,21 +189,22 @@ def get_all_options(self, drop_default=False):

    # TODO(BEAM-1319): PipelineOption sub-classes in the main session might be
    # repeated. Pick last unique instance of each subclass to avoid conflicts.


BEAM-1319 appears as fixed.

aaltay · 2017-04-18T20:50:01Z

PTAL, updated for new lint errors.

aaltay · 2017-04-18T20:50:08Z

Run Python PostCommit

sb2nov · 2017-04-18T20:53:35Z

LGTM as this was already merged

Thanks for the help.

sb2nov approved these changes Apr 18, 2017

View reviewed changes

aaltay added 5 commits April 18, 2017 13:14

Revert "Revert "Revert "Revert "Add ValueProvider class for FileBased…

888b7bc

…Source I/O Transforms"""" This reverts commit 0f5c363.

Revert "Revert "Throw specialized exception in value providers""

38a279e

This reverts commit 4854291.

Remove options_id concept from templated runs.

52843be

fix lint errors

152c7b5

Reviewer comments.

46e98bd

mariapython reviewed Apr 18, 2017

View reviewed changes

Fix new lint errors and move RuntimeValueProviderError to errors.py

675b809

aaltay force-pushed the templ branch from 9618070 to 675b809 Compare April 18, 2017 20:48

asfgit closed this in eb2e6aa Apr 18, 2017

aaltay deleted the templ branch May 12, 2017 16:50

Nielymmah mentioned this pull request Sep 18, 2023

[Snyk] Upgrade @grpc/grpc-js from 1.8.8 to 1.9.1 Nielymmah/beam#3

Open

kingjay66 mentioned this pull request Oct 14, 2023

[Snyk] Upgrade @grpc/grpc-js from 1.6.7 to 1.9.3 kingjay66/beam#1

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BEAM-802] Python templates #2545

[BEAM-802] Python templates #2545

aaltay commented Apr 14, 2017

aaltay commented Apr 14, 2017

aaltay commented Apr 14, 2017

asfbot commented Apr 14, 2017

aaltay commented Apr 14, 2017

asfbot commented Apr 14, 2017

asfbot commented Apr 14, 2017

coveralls commented Apr 15, 2017

asfbot commented Apr 15, 2017

sb2nov left a comment

sb2nov Apr 17, 2017

aaltay Apr 18, 2017

sb2nov Apr 17, 2017

aaltay Apr 18, 2017

sb2nov Apr 17, 2017

aaltay Apr 18, 2017

sb2nov Apr 17, 2017

mariapython Apr 18, 2017

aaltay Apr 18, 2017

sb2nov Apr 17, 2017

aaltay Apr 18, 2017

mariapython Apr 18, 2017

sb2nov Apr 17, 2017

aaltay Apr 18, 2017

sb2nov Apr 18, 2017

aaltay Apr 18, 2017

aaltay commented Apr 18, 2017

aaltay commented Apr 18, 2017

mariapython Apr 18, 2017

aaltay Apr 18, 2017

aaltay commented Apr 18, 2017

aaltay commented Apr 18, 2017

sb2nov commented Apr 18, 2017

		@@ -119,21 +189,22 @@ def get_all_options(self, drop_default=False):

		# TODO(BEAM-1319): PipelineOption sub-classes in the main session might be
		# repeated. Pick last unique instance of each subclass to avoid conflicts.

[BEAM-802] Python templates #2545

[BEAM-802] Python templates #2545

Conversation

aaltay commented Apr 14, 2017

aaltay commented Apr 14, 2017

aaltay commented Apr 14, 2017

asfbot commented Apr 14, 2017

aaltay commented Apr 14, 2017

asfbot commented Apr 14, 2017

Build result: FAILURE

asfbot commented Apr 14, 2017

coveralls commented Apr 15, 2017

asfbot commented Apr 15, 2017

sb2nov 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

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

aaltay commented Apr 18, 2017

aaltay commented Apr 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaltay commented Apr 18, 2017

aaltay commented Apr 18, 2017

sb2nov commented Apr 18, 2017