lostluck commented on a change in pull request #15409:
URL: https://github.com/apache/beam/pull/15409#discussion_r698613685
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -4578,6 +4579,7 @@ firings:
{{< code_sample "sdks/python/apache_beam/examples/snippets/snippets_test.py"
model_early_late_triggers >}}
{{< /highlight >}}
Review comment:
Since you've added snippets, I assume the intent is to include the
appropriate block so they get displayed:
```
{{< highlight go >}}
{{< code_sample "sdks/go/examples/snippets/09triggers.go"
model_early_late_triggers >}}
{{< /highlight >}}
```
And similar throughout.
You can see how things render using `./gradlew :website:serveWebsite ` to
stand up a localhost version of the website. Note, for new snippet files you
need to add at least one code_sample block for them to be known. I end up
iterating between using the gradle command above, stopping that and then
clearing the running docker containers `docker stop $(docker ps -q -f
ancestor=beam-website)` to see fresh changes on the next run of the gradle
command.
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -4594,7 +4596,8 @@ modifying this behavior.
The `AfterProcessingTime` trigger operates on *processing time*. For example,
the <span
class="language-java">`AfterProcessingTime.pastFirstElementInPane()`</span>
-<span class="language-py">`AfterProcessingTime`</span> trigger emits a window
+<span class="language-py">`AfterProcessingTime`</span>
+<span class="language-go">`Trigger{Kind: AfterProcessingTimeTrigger, Delay:
5000}`</span> trigger emits a window
Review comment:
can probably simplify this down to `window.AfterProcessingTimeTrigger`
which matches how users will see it. We don't need to show the wrapping, that's
what the code sample blocks are for.
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -4607,14 +4610,16 @@ window.
Beam provides one data-driven trigger,
<span class="language-java">`AfterPane.elementCountAtLeast()`</span>
-<span class="language-py">`AfterCount`</span>. This trigger works on an element
+<span class="language-py">`AfterCount`</span>
+<span class="language-go">`Trigger{Kind: ElementCountTrigger, ElementCount:
2}`</span>. This trigger works on an element
count; it fires after the current pane has collected at least *N* elements.
This
allows a window to emit early results (before all the data has accumulated),
which can be particularly useful if you are using a single global window.
It is important to note that if, for example, you specify
<span class="language-java">`.elementCountAtLeast(50)`</span>
-<span class="language-py">AfterCount(50)</span> and only 32 elements arrive,
+<span class="language-py">AfterCount(50)</span>
+<span class="language-go">`Trigger{Kind: ElementCountTrigger, ElementCount:
50}`</span> and only 32 elements arrive,
Review comment:
For contrast, this one indicates a specific number.
Note how verbose this is compared to Java and Python. Perhaps we should have
helper functions like `window.TriggerAfterCount(50)` to make the user
experience user easier? (it avoids users accidentally constructing bad triggers
too)
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -4679,6 +4695,13 @@ trigger. To set a window to discard fired panes, set
`accumulation_mode` to
`DISCARDING`.
{{< /paragraph >}}
+{{< paragraph class="language-go" >}}
+To set a window to accumulate the panes that are produced when the trigger
+fires, set the `AccumulationMode{Mode:}` parameter to
`AccumulationMode_ACCUMULATING` when you set the
Review comment:
The phrasing here is awkward. Don't try to present it as setting a
parameter in these instances, demonstrate the code directly instead.
```
To set a window to accumulate the panes that are produced when the trigger
fires, use `beam.AccumulationMode{Mode: window.Accumulating}`.
```
etc.
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -4679,6 +4695,13 @@ trigger. To set a window to discard fired panes, set
`accumulation_mode` to
`DISCARDING`.
{{< /paragraph >}}
+{{< paragraph class="language-go" >}}
+To set a window to accumulate the panes that are produced when the trigger
+fires, set the `AccumulationMode{Mode:}` parameter to
`AccumulationMode_ACCUMULATING` when you set the
Review comment:
It's not 100% clear to me whether we can make this a bit shorter for
users by having convenience methods, other than using separate convenience
method names: `beam.PanesAccumulate`, `beam.PanesDiscard`, (ignoring
`beam.PanesRetract` for now since nothing supports it IIRC...)
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -4623,7 +4628,7 @@ either when I receive 50 elements, or every 1 second".
### 9.4. Setting a trigger {#setting-a-trigger}
When you set a windowing function for a `PCollection` by using the
-<span class="language-java">`Window`</span><span
class="language-py">`WindowInto`</span>
+<span class="language-java">`Window`</span><span
class="language-py">`WindowInto`</span><span
class="language-go">`WindowInto`</span>
Review comment:
In this case, to make it look more Go, clarify directly that it's
`beam.WindowInto` since that's how users will always see it.
Note: Fun feature of CSS: If two blockers *would* have the same content, you
can just put two classes in the span class argument. eg. `<span
class="language-py language-go">.
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -4643,6 +4648,13 @@ element in that window has been processed. The
`accumulation_mode` parameter
sets the window's **accumulation mode**.
{{< /paragraph >}}
+{{< paragraph class="language-go" >}}
+You set the trigger(s) for a `PCollection` by passing in the `WindowTrigger`
parameter
Review comment:
When directly referring to code constructs for Go, prefer the way it
shows up in the code, including the usual package short name. eg.
`beam.WindowTrigger`, `beam.WindowInto`, `beam.AccumulationMode` etc. (Since
the PCollection type isn't being referred to in code, it doesn't get the
package prefix.)
##########
File path: website/www/site/content/en/documentation/programming-guide.md
##########
@@ -4607,14 +4610,16 @@ window.
Beam provides one data-driven trigger,
<span class="language-java">`AfterPane.elementCountAtLeast()`</span>
-<span class="language-py">`AfterCount`</span>. This trigger works on an element
+<span class="language-py">`AfterCount`</span>
+<span class="language-go">`Trigger{Kind: ElementCountTrigger, ElementCount:
2}`</span>. This trigger works on an element
Review comment:
Similar to the above we can probably simplify this down to
`window.ElementCountTrigger` which matches how users will see it. We don't need
to show the wrapping, that's what the code sample blocks are for.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
