You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2021/06/22 01:06:46 UTC
[GitHub] [beam] lostluck opened a new pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
lostluck opened a new pull request #15057:
URL: https://github.com/apache/beam/pull/15057
This PR adds additional Go SDK content to the Beam Programming Guide, up to the end of section 4.2. It also corrects a few incidental display errors in earlier sections (incorrect use of span wrappings or paragraph tags).
Notably an additional example package is added to hold Go snippets for the generation of website.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) 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](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
`ValidatesRunner` compliance status (on master branch)
--------------------------------------------------------
<table>
<thead>
<tr>
<th>Lang</th>
<th>ULR</th>
<th>Dataflow</th>
<th>Flink</th>
<th>Samza</th>
<th>Spark</th>
<th>Twister2</th>
</tr>
</thead>
<tbody>
<tr>
<td>Go</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon">
</a>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
<tr>
<td>Java</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_ULR/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_ULR/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Streaming/lastCompletedBuild/badge/icon?subject=V1+Streaming">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon?subject=V1+Java+11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2_Streaming/lastCompletedBuild/badge/icon?subject=V2+Streaming">
</a><br>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon?subject=Java+8">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon?subject=Java+11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon?subject=Portable+Streaming">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Samza/lastCompletedBuild/badge/icon?subject=Portable">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon?subject=Structured+Streaming">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon">
</a>
</td>
</tr>
<tr>
<td>Python</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon?subject=ValCont">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
<tr>
<td>XLang</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
</tbody>
</table>
Examples testing status on various runners
--------------------------------------------------------
<table>
<thead>
<tr>
<th>Lang</th>
<th>ULR</th>
<th>Dataflow</th>
<th>Flink</th>
<th>Samza</th>
<th>Spark</th>
<th>Twister2</th>
</tr>
</thead>
<tbody>
<tr>
<td>Go</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>Java</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Cron/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Java11_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Java11_Cron/lastCompletedBuild/badge/icon?subject=V1+Java11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_Examples_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_Examples_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>Python</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>XLang</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
</tbody>
</table>
Post-Commit SDK/Transform Integration Tests Status (on master branch)
------------------------------------------------------------------------------------------------
<table>
<thead>
<tr>
<th>Go</th>
<th>Java</th>
<th>Python</th>
</tr>
</thead>
<tbody>
<tr>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon?subject=3.6">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon?subject=3.7">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/badge/icon?subject=3.8">
</a>
</td>
</tr>
</tbody>
</table>
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
<table>
<thead>
<tr>
<th>---</th>
<th>Java</th>
<th>Python</th>
<th>Go</th>
<th>Website</th>
<th>Whitespace</th>
<th>Typescript</th>
</tr>
</thead>
<tbody>
<tr>
<td>Non-portable</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon">
</a><br>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon?subject=Tests">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon?subject=Lint">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/badge/icon?subject=Docker">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/badge/icon?subject=Docs">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
</tr>
<tr>
<td>Portable</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_GoPortable_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_GoPortable_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
</tbody>
</table>
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on pull request #15057:
URL: https://github.com/apache/beam/pull/15057#issuecomment-865447421
R: @youngoli @tysonjh
Question: What do you think of me consolidating the code example files a bit? I think they should probably not get larger than ~400 lines each, but the small ones are certainly too small. I'm thinking consolidating roughly on size so all the earlier sections would be in pipeline.go, transforms.go would cover section 4, etc.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r662656430
##########
File path: sdks/go/examples/snippets/01_03intro.go
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package snippets
+
+import (
+ "flag"
+
+ "github.com/apache/beam/sdks/go/pkg/beam"
+ "github.com/apache/beam/sdks/go/pkg/beam/io/textio"
+)
+
+// PipelineConstruction contains snippets for the initial sections of
+// the Beam Programming Guide, from initializing to submitting a
+// pipeline.
+func PipelineConstruction() {
+ // [START pipeline_options]
+ // If beamx or Go flags are used, flags must be parsed first,
+ // before beam.Init() is called.
+ flag.Parse()
+ // [END pipeline_options]
+
+ // [START pipelines_constructing_creating]
+ // beam.Init() is an initialization hook that must be called near
+ // the beginging of main().
+ beam.Init()
+
+ // Create the Pipeline object and root scope.
+ pipeline, scope := beam.NewPipelineWithRoot()
+ // [END pipelines_constructing_creating]
+
+ // [START pipelines_constructing_reading]
+ lines := textio.Read(scope, "gs://some/inputData.txt")
+ // [END pipelines_constructing_reading]
+
+ _ = []interface{}{pipeline, scope, lines}
+}
+
+// Create demonstrates using beam.CreateList.
+func Create() {
+ // [START model_pcollection]
+ lines := []string{
+ "To be, or not to be: that is the question: ",
+ "Whether 'tis nobler in the mind to suffer ",
+ "The slings and arrows of outrageous fortune, ",
+ "Or to take arms against a sea of troubles, ",
+ }
+
+ // Create the Pipeline object and root scope.
+ p, s := beam.NewPipelineWithRoot()
Review comment:
Agreed that the convention needed to be made explicit, and this was an opportunity to do it. Additional commentary added for what's happening in the call.
--
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
[GitHub] [beam] lostluck commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r662657184
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -546,6 +602,12 @@ a branching pipeline, like so:
[PCollection of 'B' names] = [PCollection of database table rows] | [Transform B]
{{< /highlight >}}
+{{< highlight go >}}
+[PCollection of database table rows] = beam.ParDo(s, [Read Transform], [Database Table Reader])
Review comment:
Done.
--
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
[GitHub] [beam] jrmccluskey commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r663110713
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -716,11 +837,45 @@ static class ComputeWordLengthFn extends DoFn<String, Integer> {
{{< code_sample "sdks/python/apache_beam/examples/snippets/snippets_test.py" model_pardo_pardo >}}
{{< /highlight >}}
-{{< paragraph class="language-java" >}}
+{{< highlight go >}}
+{{< code_sample "sdks/go/examples/snippets/04transforms.go" model_pardo_pardo >}}
+{{< /highlight >}}
+
+{{< paragraph class="language-go" >}}
+Simple DoFns can also be written as functions.
+{{< /paragraph >}}
+
+{{< highlight go >}}
+func ComputeWordLengthFn(word string, emit func(int)) { ... }
+
+func init() {
+ beam.RegisterFunction(ComputeWordLengthFn)
+}
+{{< /highlight >}}
+
+<span class="language-go" >
+
+> **Note:** Wether using a structural `DoFn` type or a functional `DoFn`, they should be registered with
Review comment:
```suggestion
> **Note:** Whether using a structural `DoFn` type or a functional `DoFn`, they should be registered with
```
--
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
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658383456
##########
File path: sdks/go/examples/snippets/04transforms.go
##########
@@ -0,0 +1,289 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package snippets
+
+import (
+ "fmt"
+ "math"
+ "reflect"
+ "sort"
+ "strings"
+
+ "github.com/apache/beam/sdks/go/pkg/beam"
+ "github.com/apache/beam/sdks/go/pkg/beam/transforms/stats"
+)
+
+// [START model_pardo_pardo]
+
+// ComputeWordLengthFn is the DoFn to perform on each element in the input PCollection.
+type ComputeWordLengthFn struct{}
+
+// ProcessElement is the method to execute for each element.
+func (fn *ComputeWordLengthFn) ProcessElement(word string, emit func(int)) {
+ emit(len(word))
+}
+
+// DoFns must be registered with beam.
+func init() {
+ beam.RegisterType(reflect.TypeOf((*ComputeWordLengthFn)(nil)))
+}
+
+// [END model_pardo_pardo]
+
+// applyWordLen applies ComputeWordLengthFn to words, which must be
+// a PCollection<string>
+func applyWordLen(s beam.Scope, words beam.PCollection) beam.PCollection {
+ // [START model_pardo_apply]
+ wordLengths := beam.ParDo(s, &ComputeWordLengthFn{}, words)
Review comment:
Suggested change:
```
wordLengths := beam.ParDo(s, &ComputeWordLengthFn{}, words)
```
to
```
wordLengths := beam.ParDo(scope, &ComputeWordLengthFn{}, words)
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on pull request #15057:
URL: https://github.com/apache/beam/pull/15057#issuecomment-872627950
PTAL
--
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
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658386948
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -546,6 +602,12 @@ a branching pipeline, like so:
[PCollection of 'B' names] = [PCollection of database table rows] | [Transform B]
{{< /highlight >}}
+{{< highlight go >}}
+[PCollection of database table rows] = beam.ParDo(s, [Read Transform], [Database Table Reader])
Review comment:
Suggested change:
```
[PCollection of database table rows] = beam.ParDo(s, [Read Transform], [Database Table Reader])
```
to
```
[PCollection of database table rows] = beam.ParDo(scope, [Read Transform], [Database Table Reader])
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r662656704
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -499,16 +531,34 @@ the transform itself as an argument, and the operation returns the output
[Output PCollection] = [Input PCollection] | [Transform]
{{< /highlight >}}
+{{< highlight go >}}
+[Output PCollection] := beam.ParDo(s, [Transform], [Input PCollection])
Review comment:
Since this and the others are abstract, it's good to call out the scope with a less abstract name. Done.
--
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
[GitHub] [beam] lostluck commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r662652429
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -716,11 +818,45 @@ static class ComputeWordLengthFn extends DoFn<String, Integer> {
{{< code_sample "sdks/python/apache_beam/examples/snippets/snippets_test.py" model_pardo_pardo >}}
{{< /highlight >}}
-{{< paragraph class="language-java" >}}
+{{< highlight go >}}
+{{< code_sample "sdks/go/examples/snippets/04transforms.go" model_pardo_pardo >}}
+{{< /highlight >}}
+
+{{< paragraph class="language-go" >}}
+Simple DoFns can also be written as functions.
+{{< /paragraph >}}
+
+{{< highlight go >}}
+func ComputeWordLengthFn(word string, emit func(int)) { ... }
+
+func init() {
+ beam.RegisterFunction(ComputeWordLengthFn)
+}
+{{< /highlight >}}
+
+<span class="language-go" >
+
+> **Note:** Wether using a structural `DoFn` type or a functional `DoFn`, they should be registered with
+> beam in an `init` block. Otherwise they may not execute on distributed runners.
+
+</span>
+
+<span class="language-java">
+
> **Note:** If the elements in your input `PCollection` are key/value pairs, you
> can access the key or value by using `element.getKey()` or
> `element.getValue()`, respectively.
-{{< /paragraph >}}
+
+</span>
+
+<span class="language-go">
+
+> **Note:** If the elements in your input `PCollection` are key/value pairs, your
+> process element method must have two parameters, for each of the key and value,
+> respectively. Similarly, key/value pairs are also output as spearate
Review comment:
Nice catch. Done.
--
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
[GitHub] [beam] lostluck commented on pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on pull request #15057:
URL: https://github.com/apache/beam/pull/15057#issuecomment-865447421
R: @youngoli @tysonjh
Question: What do you think of me consolidating the code example files a bit? I think they should probably not get larger than ~400 lines each, but the small ones are certainly too small. I'm thinking consolidating roughly on size so all the earlier sections would be in pipeline.go, transforms.go would cover section 4, etc.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658384301
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -499,16 +531,34 @@ the transform itself as an argument, and the operation returns the output
[Output PCollection] = [Input PCollection] | [Transform]
{{< /highlight >}}
+{{< highlight go >}}
+[Output PCollection] := beam.ParDo(s, [Transform], [Input PCollection])
Review comment:
Suggested change:
```
[Output PCollection] := beam.ParDo(s, [Transform], [Input PCollection])
```
to
```
[Output PCollection] := beam.ParDo(scope, [Transform], [Input PCollection])
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r659105880
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -522,6 +572,12 @@ together transforms to create a pipeline that successively modifies input data:
| [Third Transform])
{{< /highlight >}}
+{{< highlight go >}}
+[Second PCollection] := beam.ParDo(s, [First Transform], [Initial Input PCollection])
+[Third PCollection] := beam.ParDo(s, [Second Transform], [Second PCollection])
Review comment:
I think the abbreviation makes sense when one is looking at the whole code. The rendering on the online template breaks context. It may be my own issues with reading when dealing with context shift so I'm probably not the best judge for the average audience. Thank you for listening to the comment though.
--
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
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658391044
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -675,10 +746,31 @@ the types of the input and output elements. If your `DoFn` processes incoming
look like this:
{{< /paragraph >}}
+{{< paragraph class="language-go" >}}
+A `DoFn` processes one element at a time from the input `PCollection`. When you
Review comment:
Suggested change:
A `DoFn` processes one element at a time from the input `PCollection`. When you
to
A `DoFn` processes one element at a time from the input `PCollection`. Even though `Fn` is part of the abbreviation, suggestion it has to be a function, you can provide either a `struct` or a `func`. When you
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r659043365
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -522,6 +572,12 @@ together transforms to create a pipeline that successively modifies input data:
| [Third Transform])
{{< /highlight >}}
+{{< highlight go >}}
+[Second PCollection] := beam.ParDo(s, [First Transform], [Initial Input PCollection])
+[Third PCollection] := beam.ParDo(s, [Second Transform], [Second PCollection])
Review comment:
This one is reasonable since it's abstract code anyway. However, I'd rather not force a generation of Go SDK users to repeatedly use `scope` for a pervasive variable. Eventually too much such repetition is not useful and draws focus from the distinct parts of the call.
I agree that towards the pipeline phase, we should explain the scope variable better, what it's for (referring to the section on composites), and that conventionally, it's abbreviated to 's' which is like how `context.Context` is abbreviated to `ctx`, and `io.Reader` and `io.Writer` become `r` and `w` respectively.
--
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
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658373954
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -171,9 +186,13 @@ This interprets command-line arguments that follow the format:
--<option>=<value>
```
-> **Note:** Appending the method `.withValidation` will check for required
+<span class="language-java">
+
+> Appending the method `.withValidation` will check for required
> command-line arguments and validate argument values.
+</span>
+
Building your `PipelineOptions` this way lets you specify any of the options as
Review comment:
Should there be:
```
<span class="language-java language-py">
Building your `PipelineOptions` this way lets you specify any of the options as a command-line argument.
</span>
<span class="language-go">
Defining flag variables this way lets you specify any of the options as a command-line argument.
</span>
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658372679
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -171,9 +186,13 @@ This interprets command-line arguments that follow the format:
--<option>=<value>
```
-> **Note:** Appending the method `.withValidation` will check for required
+<span class="language-java">
+
+> Appending the method `.withValidation` will check for required
> command-line arguments and validate argument values.
+</span>
+
Building your `PipelineOptions` this way lets you specify any of the options as
Review comment:
Does highlighting `PipelineOptions` in the guide for go developers, refer specifically to the var [`PipelineOptions`](https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/forward.go#L157). Should it not be mentioned for go developers reading this guide, particularly new developers? I can see why it was included for the Java documentation.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658378464
##########
File path: sdks/go/examples/snippets/01_03intro.go
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package snippets
+
+import (
+ "flag"
+
+ "github.com/apache/beam/sdks/go/pkg/beam"
+ "github.com/apache/beam/sdks/go/pkg/beam/io/textio"
+)
+
+// PipelineConstruction contains snippets for the initial sections of
+// the Beam Programming Guide, from initializing to submitting a
+// pipeline.
+func PipelineConstruction() {
+ // [START pipeline_options]
+ // If beamx or Go flags are used, flags must be parsed first,
+ // before beam.Init() is called.
+ flag.Parse()
+ // [END pipeline_options]
+
+ // [START pipelines_constructing_creating]
+ // beam.Init() is an initialization hook that must be called near
+ // the beginging of main().
+ beam.Init()
+
+ // Create the Pipeline object and root scope.
+ pipeline, scope := beam.NewPipelineWithRoot()
+ // [END pipelines_constructing_creating]
+
+ // [START pipelines_constructing_reading]
+ lines := textio.Read(scope, "gs://some/inputData.txt")
Review comment:
Suggested comment insertion:
```
// Read the file at the URI 'gs://some/inputData.txt' and return the lines as a PCollection<string>.
// Notice the scope as the first variable when calling the method as is needed when calling all transforms.
lines := textio.Read(scope, "gs://some/inputData.txt")
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r662653900
##########
File path: sdks/go/examples/snippets/01_03intro.go
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package snippets
+
+import (
+ "flag"
+
+ "github.com/apache/beam/sdks/go/pkg/beam"
+ "github.com/apache/beam/sdks/go/pkg/beam/io/textio"
+)
+
+// PipelineConstruction contains snippets for the initial sections of
+// the Beam Programming Guide, from initializing to submitting a
+// pipeline.
+func PipelineConstruction() {
+ // [START pipeline_options]
+ // If beamx or Go flags are used, flags must be parsed first,
+ // before beam.Init() is called.
+ flag.Parse()
+ // [END pipeline_options]
+
+ // [START pipelines_constructing_creating]
+ // beam.Init() is an initialization hook that must be called near
+ // the beginging of main().
Review comment:
Good suggestion. Simplified the tail to "before creating a pipeline."
--
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
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658386639
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -522,6 +572,12 @@ together transforms to create a pipeline that successively modifies input data:
| [Third Transform])
{{< /highlight >}}
+{{< highlight go >}}
+[Second PCollection] := beam.ParDo(s, [First Transform], [Initial Input PCollection])
+[Third PCollection] := beam.ParDo(s, [Second Transform], [Second PCollection])
+[Final Output PCollection] := beam.ParDo(s, [Third Transform], [Third PCollection])
Review comment:
Suggested change:
```
[Final Output PCollection] := beam.ParDo(s, [Third Transform], [Third PCollection])
```
to
```
[Final Output PCollection] := beam.ParDo(scope, [Third Transform], [Third PCollection])
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658391044
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -675,10 +746,31 @@ the types of the input and output elements. If your `DoFn` processes incoming
look like this:
{{< /paragraph >}}
+{{< paragraph class="language-go" >}}
+A `DoFn` processes one element at a time from the input `PCollection`. When you
Review comment:
Suggested change:
A `DoFn` processes one element at a time from the input `PCollection`. When you
to
A `DoFn` processes one element at a time from the input `PCollection`. A `DoFn` can be either a `struct` or a `func`. When you
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tysonjh commented on pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
tysonjh commented on pull request #15057:
URL: https://github.com/apache/beam/pull/15057#issuecomment-868828534
> R: @youngoli @tysonjh
>
> Question: What do you think of me consolidating the code example files a bit? I think they should probably not get larger than ~400 lines each, but the small ones are certainly too small. I'm thinking consolidating roughly on size so all the earlier sections would be in pipeline.go, transforms.go would cover section 4, etc.
This makes sense to me. Does this impact how it will be displayed in docs? E.g. using github snippets
--
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
[GitHub] [beam] youngoli commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
youngoli commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r660326406
##########
File path: sdks/go/examples/snippets/04transforms.go
##########
@@ -0,0 +1,289 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package snippets
+
+import (
+ "fmt"
+ "math"
+ "reflect"
+ "sort"
+ "strings"
+
+ "github.com/apache/beam/sdks/go/pkg/beam"
+ "github.com/apache/beam/sdks/go/pkg/beam/transforms/stats"
+)
+
+// [START model_pardo_pardo]
+
+// ComputeWordLengthFn is the DoFn to perform on each element in the input PCollection.
+type ComputeWordLengthFn struct{}
+
+// ProcessElement is the method to execute for each element.
+func (fn *ComputeWordLengthFn) ProcessElement(word string, emit func(int)) {
Review comment:
As the first example of a ParDo, would it be better to go with returning outputs instead of using an emit function? I think there's a section somewhere in the programming guide for outputting multiple elements per input where we can introduce emit functions.
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -499,16 +531,34 @@ the transform itself as an argument, and the operation returns the output
[Output PCollection] = [Input PCollection] | [Transform]
{{< /highlight >}}
+{{< highlight go >}}
+[Output PCollection] := beam.ParDo(s, [Transform], [Input PCollection])
+{{< /highlight >}}
+
+{{< paragraph class="language-java language-py" >}}
Because Beam uses a generic `apply` method for `PCollection`, you can both chain
transforms sequentially and also apply transforms that contain other transforms
nested within (called [composite transforms](#composite-transforms) in the Beam
SDKs).
+{{< /paragraph >}}
+
+{{< paragraph class="language-go" >}}
+Because Go doesn't support function overloading, it's recommended to
Review comment:
We can probably remove `"Because Go doesn't support function overloading..."` and leave just the recommendation. Users don't really need to understand the technical details of why took this approach, just what to do.
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -716,11 +818,45 @@ static class ComputeWordLengthFn extends DoFn<String, Integer> {
{{< code_sample "sdks/python/apache_beam/examples/snippets/snippets_test.py" model_pardo_pardo >}}
{{< /highlight >}}
-{{< paragraph class="language-java" >}}
+{{< highlight go >}}
+{{< code_sample "sdks/go/examples/snippets/04transforms.go" model_pardo_pardo >}}
+{{< /highlight >}}
+
+{{< paragraph class="language-go" >}}
+Simple DoFns can also be written as functions.
+{{< /paragraph >}}
+
+{{< highlight go >}}
+func ComputeWordLengthFn(word string, emit func(int)) { ... }
+
+func init() {
+ beam.RegisterFunction(ComputeWordLengthFn)
+}
+{{< /highlight >}}
+
+<span class="language-go" >
+
+> **Note:** Wether using a structural `DoFn` type or a functional `DoFn`, they should be registered with
+> beam in an `init` block. Otherwise they may not execute on distributed runners.
+
+</span>
+
+<span class="language-java">
+
> **Note:** If the elements in your input `PCollection` are key/value pairs, you
> can access the key or value by using `element.getKey()` or
> `element.getValue()`, respectively.
-{{< /paragraph >}}
+
+</span>
+
+<span class="language-go">
+
+> **Note:** If the elements in your input `PCollection` are key/value pairs, your
+> process element method must have two parameters, for each of the key and value,
+> respectively. Similarly, key/value pairs are also output as spearate
Review comment:
```suggestion
> respectively. Similarly, key/value pairs are also output as separate
```
##########
File path: sdks/go/examples/snippets/01_03intro.go
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package snippets
+
+import (
+ "flag"
+
+ "github.com/apache/beam/sdks/go/pkg/beam"
+ "github.com/apache/beam/sdks/go/pkg/beam/io/textio"
+)
+
+// PipelineConstruction contains snippets for the initial sections of
+// the Beam Programming Guide, from initializing to submitting a
+// pipeline.
+func PipelineConstruction() {
+ // [START pipeline_options]
+ // If beamx or Go flags are used, flags must be parsed first,
+ // before beam.Init() is called.
+ flag.Parse()
+ // [END pipeline_options]
+
+ // [START pipelines_constructing_creating]
+ // beam.Init() is an initialization hook that must be called near
+ // the beginging of main().
+ beam.Init()
+
+ // Create the Pipeline object and root scope.
+ pipeline, scope := beam.NewPipelineWithRoot()
+ // [END pipelines_constructing_creating]
+
+ // [START pipelines_constructing_reading]
+ lines := textio.Read(scope, "gs://some/inputData.txt")
+ // [END pipelines_constructing_reading]
+
+ _ = []interface{}{pipeline, scope, lines}
+}
+
+// Create demonstrates using beam.CreateList.
+func Create() {
+ // [START model_pcollection]
+ lines := []string{
+ "To be, or not to be: that is the question: ",
+ "Whether 'tis nobler in the mind to suffer ",
+ "The slings and arrows of outrageous fortune, ",
+ "Or to take arms against a sea of troubles, ",
+ }
+
+ // Create the Pipeline object and root scope.
+ p, s := beam.NewPipelineWithRoot()
Review comment:
It might be relevant to point out that using `s` as a variable name for the scope is the convention in the Go SDK. This is what the user will see if they read any other Go SDK examples or code. I'm not sure if the value of clarity outweighs sticking to convention, just wanted to mention that. What do you think?
--
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
[GitHub] [beam] lostluck commented on pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on pull request #15057:
URL: https://github.com/apache/beam/pull/15057#issuecomment-868836843
> > R: @youngoli @tysonjh
> > Question: What do you think of me consolidating the code example files a bit? I think they should probably not get larger than ~400 lines each, but the small ones are certainly too small. I'm thinking consolidating roughly on size so all the earlier sections would be in pipeline.go, transforms.go would cover section 4, etc.
>
> This makes sense to me. Does this impact how it will be displayed in docs? E.g. using github snippets
If one looked at the GoDoc for the snippet package, no that wouldn't display correctly at all. GoDoc doesn't take file ordering into account everything is sorted by top level name (so methods are grouped with their type as you'd expect). I don't know about github snippets.
As for using those snippets in the beam website, the snippets are file origin agnostic, and copied at static site generation, so how they're stored in code doesn't matter.
Overall, the code organization is largely cosmetic for the code itself, so the present approach of prefixing with BPG section number means the files will be in the order that match the section whose code they contain. Everything internally is magic comments for the code snippet puller.
--
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
[GitHub] [beam] lostluck commented on pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on pull request #15057:
URL: https://github.com/apache/beam/pull/15057#issuecomment-872628353
R: @jrmccluskey
--
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
[GitHub] [beam] damondouglas commented on pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on pull request #15057:
URL: https://github.com/apache/beam/pull/15057#issuecomment-867863937
@lostluck just letting you know I got the notification and this is on my radar. Thank you for inviting me to look at this.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658365765
##########
File path: sdks/go/examples/snippets/01_03intro.go
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package snippets
+
+import (
+ "flag"
+
+ "github.com/apache/beam/sdks/go/pkg/beam"
+ "github.com/apache/beam/sdks/go/pkg/beam/io/textio"
+)
+
+// PipelineConstruction contains snippets for the initial sections of
+// the Beam Programming Guide, from initializing to submitting a
+// pipeline.
+func PipelineConstruction() {
+ // [START pipeline_options]
+ // If beamx or Go flags are used, flags must be parsed first,
+ // before beam.Init() is called.
+ flag.Parse()
+ // [END pipeline_options]
+
+ // [START pipelines_constructing_creating]
+ // beam.Init() is an initialization hook that must be called near
+ // the beginging of main().
Review comment:
I've been reading this remembering what it was like to start beam for the first time. I thought perhaps this suggested change might be more explicit.
Suggested change:
```
// beam.Init() is an initialization hook that must be called near
// the beginning of main().
```
to
```
// beam.Init() is an initialization hook that must be called inside the main() method
// before calling NewPipeline or NewPipelineWithRoot
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on pull request #15057:
URL: https://github.com/apache/beam/pull/15057#issuecomment-867816755
R: @damondouglas
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on pull request #15057:
URL: https://github.com/apache/beam/pull/15057#issuecomment-866371157
Definitely helped things to move them into section number prefixed files. The code is roughly in order as it appears in the BPG (some things are more naturally in the test file, rather than in the library file). The transform code is all tested against the Go Direct Runner (these simple cases are expected to work against it). Even with nothing the the into examples direcly tested, we now have 80%+ unit tested.
This format will be maintained for subsequent PRs. I'll be branching from here and carrying on with the rest of the BPG, hopefully in smaller chunks.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r662654337
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -171,9 +186,13 @@ This interprets command-line arguments that follow the format:
--<option>=<value>
```
-> **Note:** Appending the method `.withValidation` will check for required
+<span class="language-java">
+
+> Appending the method `.withValidation` will check for required
> command-line arguments and validate argument values.
+</span>
+
Building your `PipelineOptions` this way lets you specify any of the options as
Review comment:
Done.
--
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
[GitHub] [beam] lostluck commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r662660096
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -675,10 +746,31 @@ the types of the input and output elements. If your `DoFn` processes incoming
look like this:
{{< /paragraph >}}
+{{< paragraph class="language-go" >}}
+A `DoFn` processes one element at a time from the input `PCollection`. When you
+create a `DoFn` struct, you'll need to provide type parameters that match
+the types of the input and output elements in a ProcessElement method.
+If your `DoFn` processes incoming `string` elements and produces `int` elements
+for the output collection (like our previous example, `ComputeWordLengthFn`), your dofn could
+look like this:
+{{< /paragraph >}}
+
{{< highlight java >}}
static class ComputeWordLengthFn extends DoFn<String, Integer> { ... }
{{< /highlight >}}
+{{< highlight go >}}
+type ComputeWordLengthFn struct{}
Review comment:
I was 50/50 on including full Go comment in some of the examples. Since this is first one it does make sense in some way to include them. I worry that they could be a distracting detail though.
--
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
[GitHub] [beam] lostluck commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r662649063
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -499,16 +531,34 @@ the transform itself as an argument, and the operation returns the output
[Output PCollection] = [Input PCollection] | [Transform]
{{< /highlight >}}
+{{< highlight go >}}
+[Output PCollection] := beam.ParDo(s, [Transform], [Input PCollection])
+{{< /highlight >}}
+
+{{< paragraph class="language-java language-py" >}}
Because Beam uses a generic `apply` method for `PCollection`, you can both chain
transforms sequentially and also apply transforms that contain other transforms
nested within (called [composite transforms](#composite-transforms) in the Beam
SDKs).
+{{< /paragraph >}}
+
+{{< paragraph class="language-go" >}}
+Because Go doesn't support function overloading, it's recommended to
Review comment:
Agreed. Done.
--
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
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658397674
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -675,10 +746,31 @@ the types of the input and output elements. If your `DoFn` processes incoming
look like this:
{{< /paragraph >}}
+{{< paragraph class="language-go" >}}
+A `DoFn` processes one element at a time from the input `PCollection`. When you
+create a `DoFn` struct, you'll need to provide type parameters that match
+the types of the input and output elements in a ProcessElement method.
+If your `DoFn` processes incoming `string` elements and produces `int` elements
+for the output collection (like our previous example, `ComputeWordLengthFn`), your dofn could
+look like this:
+{{< /paragraph >}}
+
{{< highlight java >}}
static class ComputeWordLengthFn extends DoFn<String, Integer> { ... }
{{< /highlight >}}
+{{< highlight go >}}
+type ComputeWordLengthFn struct{}
+
+func (fn *ComputeWordLengthFn) ProcessElement(word string, emit func(int)) {
Review comment:
Suggested edit:
```
func (fn *ComputeWordLengthFn) ProcessElement(word string, emit func(int)) {
```
to
```
// ProcessElement computes the length of word and emits the result
// When creating structs as a DoFn, the ProcessElement performs the work of this step in the pipeline
func (fn *ComputeWordLengthFn) ProcessElement(word string, emit func(int)) {
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r662660744
##########
File path: sdks/go/examples/snippets/04transforms.go
##########
@@ -0,0 +1,289 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package snippets
+
+import (
+ "fmt"
+ "math"
+ "reflect"
+ "sort"
+ "strings"
+
+ "github.com/apache/beam/sdks/go/pkg/beam"
+ "github.com/apache/beam/sdks/go/pkg/beam/transforms/stats"
+)
+
+// [START model_pardo_pardo]
+
+// ComputeWordLengthFn is the DoFn to perform on each element in the input PCollection.
+type ComputeWordLengthFn struct{}
+
+// ProcessElement is the method to execute for each element.
+func (fn *ComputeWordLengthFn) ProcessElement(word string, emit func(int)) {
Review comment:
It mostly comes down to if a user only learns the return convenience, they're stuck with 1:1. If they learn emitters, they can build off of that for 0 to 1, and multiple output PCollections by themselves.
With the return convenience, it needs to be explained that it's always the first output PCollection, in the case the return and the emitter are mixed.
--
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
[GitHub] [beam] lostluck commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r662659433
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -675,10 +746,31 @@ the types of the input and output elements. If your `DoFn` processes incoming
look like this:
{{< /paragraph >}}
+{{< paragraph class="language-go" >}}
+A `DoFn` processes one element at a time from the input `PCollection`. When you
Review comment:
I'm going to push back on this at present.
The lack of func nature is apparent in the example, immediately adjacent to the text, and shouldn't be over explained right here. At least, not at this point in the document. There's an entire section on syntactic sugars and convenience abbreviations for DoFns, which introduces that funcs can be DoFns too. I've introduced functional DoFns there since they are largely conveniences, vs the Structural DoFns.
--
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
[GitHub] [beam] lostluck commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r662655370
##########
File path: sdks/go/examples/snippets/01_03intro.go
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package snippets
+
+import (
+ "flag"
+
+ "github.com/apache/beam/sdks/go/pkg/beam"
+ "github.com/apache/beam/sdks/go/pkg/beam/io/textio"
+)
+
+// PipelineConstruction contains snippets for the initial sections of
+// the Beam Programming Guide, from initializing to submitting a
+// pipeline.
+func PipelineConstruction() {
+ // [START pipeline_options]
+ // If beamx or Go flags are used, flags must be parsed first,
+ // before beam.Init() is called.
+ flag.Parse()
+ // [END pipeline_options]
+
+ // [START pipelines_constructing_creating]
+ // beam.Init() is an initialization hook that must be called near
+ // the beginging of main().
+ beam.Init()
+
+ // Create the Pipeline object and root scope.
+ pipeline, scope := beam.NewPipelineWithRoot()
+ // [END pipelines_constructing_creating]
+
+ // [START pipelines_constructing_reading]
+ lines := textio.Read(scope, "gs://some/inputData.txt")
Review comment:
Done. Agreed that this is probably the best, first place to mention what the scope variable is for in code.
I've added a similar addendum to the Overview talking about the `Scope` variable explicitly, and that we abbreviate it to `s` conventionally. At that point I use the convention.
--
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
[GitHub] [beam] lostluck commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r662657005
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -522,6 +572,12 @@ together transforms to create a pipeline that successively modifies input data:
| [Third Transform])
{{< /highlight >}}
+{{< highlight go >}}
+[Second PCollection] := beam.ParDo(s, [First Transform], [Initial Input PCollection])
+[Third PCollection] := beam.ParDo(s, [Second Transform], [Second PCollection])
Review comment:
Re-commenting, since this is done.
--
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
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658395186
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -675,10 +746,31 @@ the types of the input and output elements. If your `DoFn` processes incoming
look like this:
{{< /paragraph >}}
+{{< paragraph class="language-go" >}}
+A `DoFn` processes one element at a time from the input `PCollection`. When you
+create a `DoFn` struct, you'll need to provide type parameters that match
+the types of the input and output elements in a ProcessElement method.
+If your `DoFn` processes incoming `string` elements and produces `int` elements
+for the output collection (like our previous example, `ComputeWordLengthFn`), your dofn could
+look like this:
+{{< /paragraph >}}
+
{{< highlight java >}}
static class ComputeWordLengthFn extends DoFn<String, Integer> { ... }
{{< /highlight >}}
+{{< highlight go >}}
+type ComputeWordLengthFn struct{}
Review comment:
Suggested change:
```
type ComputeWordLengthFn struct{}
```
to
```
// ComputeWordLengthFn is a DoFn that computes the word length of string elements.
type ComputeWordLengthFn struct{}
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658381426
##########
File path: sdks/go/examples/snippets/01_03intro.go
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package snippets
+
+import (
+ "flag"
+
+ "github.com/apache/beam/sdks/go/pkg/beam"
+ "github.com/apache/beam/sdks/go/pkg/beam/io/textio"
+)
+
+// PipelineConstruction contains snippets for the initial sections of
+// the Beam Programming Guide, from initializing to submitting a
+// pipeline.
+func PipelineConstruction() {
+ // [START pipeline_options]
+ // If beamx or Go flags are used, flags must be parsed first,
+ // before beam.Init() is called.
+ flag.Parse()
+ // [END pipeline_options]
+
+ // [START pipelines_constructing_creating]
+ // beam.Init() is an initialization hook that must be called near
+ // the beginging of main().
+ beam.Init()
+
+ // Create the Pipeline object and root scope.
+ pipeline, scope := beam.NewPipelineWithRoot()
+ // [END pipelines_constructing_creating]
+
+ // [START pipelines_constructing_reading]
+ lines := textio.Read(scope, "gs://some/inputData.txt")
+ // [END pipelines_constructing_reading]
+
+ _ = []interface{}{pipeline, scope, lines}
+}
+
+// Create demonstrates using beam.CreateList.
+func Create() {
+ // [START model_pcollection]
+ lines := []string{
+ "To be, or not to be: that is the question: ",
+ "Whether 'tis nobler in the mind to suffer ",
+ "The slings and arrows of outrageous fortune, ",
+ "Or to take arms against a sea of troubles, ",
+ }
+
+ // Create the Pipeline object and root scope.
+ p, s := beam.NewPipelineWithRoot()
Review comment:
Suggested change. In this example when it renders on the documentation, the reader is able to know that the `s` is the scope variable. However, in other examples it isn't as clear.
```
p, s := beam.NewPipelineWithRoot()
linesPCol := beam.CreateList(s, lines)
```
to
```
pipeline, scope := beam.NewPipelineWithRoot()
linesPCol := beam.CreateList(scope, lines)
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r659040850
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -265,7 +278,9 @@ Now your pipeline can accept `--input=value` and `--output=value` as command-lin
## 3. PCollections {#pcollections}
The <span class="language-java">[PCollection](https://beam.apache.org/releases/javadoc/{{< param release_latest >}}/index.html?org/apache/beam/sdk/values/PCollection.html)</span>
-<span class="language-py">`PCollection`</span> abstraction represents a
+<span class="language-py">`PCollection`</span>
+<span class="language-go">[PCollection](https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/pcollection.go#L39)</span>
Review comment:
Correct! Until the license issue is resolved though, linking to the code with marginal precision is better than nothing.
--
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
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658386068
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -522,6 +572,12 @@ together transforms to create a pipeline that successively modifies input data:
| [Third Transform])
{{< /highlight >}}
+{{< highlight go >}}
+[Second PCollection] := beam.ParDo(s, [First Transform], [Initial Input PCollection])
Review comment:
Suggested change:
```
[Second PCollection] := beam.ParDo(s, [First Transform], [Initial Input PCollection])
```
to
```
[Second PCollection] := beam.ParDo(scope, [First Transform], [Initial Input PCollection])
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658383004
##########
File path: sdks/go/examples/snippets/04transforms.go
##########
@@ -0,0 +1,289 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package snippets
+
+import (
+ "fmt"
+ "math"
+ "reflect"
+ "sort"
+ "strings"
+
+ "github.com/apache/beam/sdks/go/pkg/beam"
+ "github.com/apache/beam/sdks/go/pkg/beam/transforms/stats"
+)
+
+// [START model_pardo_pardo]
+
+// ComputeWordLengthFn is the DoFn to perform on each element in the input PCollection.
+type ComputeWordLengthFn struct{}
+
+// ProcessElement is the method to execute for each element.
+func (fn *ComputeWordLengthFn) ProcessElement(word string, emit func(int)) {
+ emit(len(word))
+}
+
+// DoFns must be registered with beam.
+func init() {
+ beam.RegisterType(reflect.TypeOf((*ComputeWordLengthFn)(nil)))
+}
+
+// [END model_pardo_pardo]
+
+// applyWordLen applies ComputeWordLengthFn to words, which must be
+// a PCollection<string>
+func applyWordLen(s beam.Scope, words beam.PCollection) beam.PCollection {
Review comment:
Suggested change so that the full `scope` variable name shows up in the snippet online instead of `s` so that it is clear to the reader that it is the `Scope` type.
```
func applyWordLen(s beam.Scope, words beam.PCollection) beam.PCollection {
```
to
```
func applyWordLen(scope beam.Scope, words beam.PCollection) beam.PCollection {
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658386398
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -522,6 +572,12 @@ together transforms to create a pipeline that successively modifies input data:
| [Third Transform])
{{< /highlight >}}
+{{< highlight go >}}
+[Second PCollection] := beam.ParDo(s, [First Transform], [Initial Input PCollection])
+[Third PCollection] := beam.ParDo(s, [Second Transform], [Second PCollection])
Review comment:
Suggested change:
```
[Third PCollection] := beam.ParDo(s, [Second Transform], [Second PCollection])
```
to
```
[Third PCollection] := beam.ParDo(scope, [Second Transform], [Second PCollection])
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
damondouglas commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r658375536
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -265,7 +278,9 @@ Now your pipeline can accept `--input=value` and `--output=value` as command-lin
## 3. PCollections {#pcollections}
The <span class="language-java">[PCollection](https://beam.apache.org/releases/javadoc/{{< param release_latest >}}/index.html?org/apache/beam/sdk/values/PCollection.html)</span>
-<span class="language-py">`PCollection`</span> abstraction represents a
+<span class="language-py">`PCollection`</span>
+<span class="language-go">[PCollection](https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/pcollection.go#L39)</span>
Review comment:
In the future, when pkg.go.dev renders the documentation, should this be the URL? https://pkg.go.dev/github.com/apache/beam/sdks/go/pkg/beam#PCollection
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] youngoli commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
youngoli commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r660326406
##########
File path: sdks/go/examples/snippets/04transforms.go
##########
@@ -0,0 +1,289 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package snippets
+
+import (
+ "fmt"
+ "math"
+ "reflect"
+ "sort"
+ "strings"
+
+ "github.com/apache/beam/sdks/go/pkg/beam"
+ "github.com/apache/beam/sdks/go/pkg/beam/transforms/stats"
+)
+
+// [START model_pardo_pardo]
+
+// ComputeWordLengthFn is the DoFn to perform on each element in the input PCollection.
+type ComputeWordLengthFn struct{}
+
+// ProcessElement is the method to execute for each element.
+func (fn *ComputeWordLengthFn) ProcessElement(word string, emit func(int)) {
Review comment:
As the first example of a ParDo, would it be better to go with returning outputs instead of using an emit function? I think there's a section somewhere in the programming guide for outputting multiple elements per input where we can introduce emit functions.
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -499,16 +531,34 @@ the transform itself as an argument, and the operation returns the output
[Output PCollection] = [Input PCollection] | [Transform]
{{< /highlight >}}
+{{< highlight go >}}
+[Output PCollection] := beam.ParDo(s, [Transform], [Input PCollection])
+{{< /highlight >}}
+
+{{< paragraph class="language-java language-py" >}}
Because Beam uses a generic `apply` method for `PCollection`, you can both chain
transforms sequentially and also apply transforms that contain other transforms
nested within (called [composite transforms](#composite-transforms) in the Beam
SDKs).
+{{< /paragraph >}}
+
+{{< paragraph class="language-go" >}}
+Because Go doesn't support function overloading, it's recommended to
Review comment:
We can probably remove `"Because Go doesn't support function overloading..."` and leave just the recommendation. Users don't really need to understand the technical details of why took this approach, just what to do.
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -716,11 +818,45 @@ static class ComputeWordLengthFn extends DoFn<String, Integer> {
{{< code_sample "sdks/python/apache_beam/examples/snippets/snippets_test.py" model_pardo_pardo >}}
{{< /highlight >}}
-{{< paragraph class="language-java" >}}
+{{< highlight go >}}
+{{< code_sample "sdks/go/examples/snippets/04transforms.go" model_pardo_pardo >}}
+{{< /highlight >}}
+
+{{< paragraph class="language-go" >}}
+Simple DoFns can also be written as functions.
+{{< /paragraph >}}
+
+{{< highlight go >}}
+func ComputeWordLengthFn(word string, emit func(int)) { ... }
+
+func init() {
+ beam.RegisterFunction(ComputeWordLengthFn)
+}
+{{< /highlight >}}
+
+<span class="language-go" >
+
+> **Note:** Wether using a structural `DoFn` type or a functional `DoFn`, they should be registered with
+> beam in an `init` block. Otherwise they may not execute on distributed runners.
+
+</span>
+
+<span class="language-java">
+
> **Note:** If the elements in your input `PCollection` are key/value pairs, you
> can access the key or value by using `element.getKey()` or
> `element.getValue()`, respectively.
-{{< /paragraph >}}
+
+</span>
+
+<span class="language-go">
+
+> **Note:** If the elements in your input `PCollection` are key/value pairs, your
+> process element method must have two parameters, for each of the key and value,
+> respectively. Similarly, key/value pairs are also output as spearate
Review comment:
```suggestion
> respectively. Similarly, key/value pairs are also output as separate
```
##########
File path: sdks/go/examples/snippets/01_03intro.go
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package snippets
+
+import (
+ "flag"
+
+ "github.com/apache/beam/sdks/go/pkg/beam"
+ "github.com/apache/beam/sdks/go/pkg/beam/io/textio"
+)
+
+// PipelineConstruction contains snippets for the initial sections of
+// the Beam Programming Guide, from initializing to submitting a
+// pipeline.
+func PipelineConstruction() {
+ // [START pipeline_options]
+ // If beamx or Go flags are used, flags must be parsed first,
+ // before beam.Init() is called.
+ flag.Parse()
+ // [END pipeline_options]
+
+ // [START pipelines_constructing_creating]
+ // beam.Init() is an initialization hook that must be called near
+ // the beginging of main().
+ beam.Init()
+
+ // Create the Pipeline object and root scope.
+ pipeline, scope := beam.NewPipelineWithRoot()
+ // [END pipelines_constructing_creating]
+
+ // [START pipelines_constructing_reading]
+ lines := textio.Read(scope, "gs://some/inputData.txt")
+ // [END pipelines_constructing_reading]
+
+ _ = []interface{}{pipeline, scope, lines}
+}
+
+// Create demonstrates using beam.CreateList.
+func Create() {
+ // [START model_pcollection]
+ lines := []string{
+ "To be, or not to be: that is the question: ",
+ "Whether 'tis nobler in the mind to suffer ",
+ "The slings and arrows of outrageous fortune, ",
+ "Or to take arms against a sea of troubles, ",
+ }
+
+ // Create the Pipeline object and root scope.
+ p, s := beam.NewPipelineWithRoot()
Review comment:
It might be relevant to point out that using `s` as a variable name for the scope is the convention in the Go SDK. This is what the user will see if they read any other Go SDK examples or code. I'm not sure if the value of clarity outweighs sticking to convention, just wanted to mention that. What do you think?
--
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
[GitHub] [beam] lostluck commented on a change in pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15057:
URL: https://github.com/apache/beam/pull/15057#discussion_r662652348
##########
File path: sdks/go/examples/snippets/04transforms.go
##########
@@ -0,0 +1,289 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package snippets
+
+import (
+ "fmt"
+ "math"
+ "reflect"
+ "sort"
+ "strings"
+
+ "github.com/apache/beam/sdks/go/pkg/beam"
+ "github.com/apache/beam/sdks/go/pkg/beam/transforms/stats"
+)
+
+// [START model_pardo_pardo]
+
+// ComputeWordLengthFn is the DoFn to perform on each element in the input PCollection.
+type ComputeWordLengthFn struct{}
+
+// ProcessElement is the method to execute for each element.
+func (fn *ComputeWordLengthFn) ProcessElement(word string, emit func(int)) {
Review comment:
I'm largely basing it on the Java approach which has the output context object, rather than the python one.
Generally users will need to know the arbitrary emitters, for more complex DoFns. The simple return case is a convenience but 1:1 is incredibly limiting. If users don't find it, they won't hurt themselves by not knowing it.
The section is outputting multiple PCollections, not multiple elements within a PCollection.
Similarly, that's why I didn't introduce it using a functional DoFn. It leads to users assuming closures can work for configuration, which can't work in Go at the moment. Structural DoFns are not the most convenient, but they are the most versatile. They prevent errors like closure functions, which are far harder to debug when they sometimes work and fail otherwise.
--
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
[GitHub] [beam] lostluck merged pull request #15057: [BEAM-12513] Update initial sections of BPG for Go
Posted by GitBox <gi...@apache.org>.
lostluck merged pull request #15057:
URL: https://github.com/apache/beam/pull/15057
--
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