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 2022/04/29 19:19:30 UTC
[GitHub] [beam] lostluck commented on a diff in pull request #17476: [BEAM-11105] Add timestamp observing watermark estimation
lostluck commented on code in PR #17476:
URL: https://github.com/apache/beam/pull/17476#discussion_r862082099
##########
sdks/go/pkg/beam/core/sdf/sdf.go:
##########
@@ -96,3 +98,12 @@ type WatermarkEstimator interface {
// splits or checkpoints to advance the output watermark of the restriction's stage.
CurrentWatermark() time.Time
}
+
+// ToWatermarkEstimator is an interface used to represent a user defined watermark estimator that
+// has the ability to observe timestamps of elements outputted from a ParDo's emit function.
+type ToWatermarkEstimator interface {
Review Comment:
If TO is supposed to be an acronym for TimestampObserving, please capitalize it accordingly.
I'd also recommend simplifying it to `TimestampObserver`. Yes it only applies to WatermarkEstimators with the embed, but that can be clarified in the type documentation, rather than the whole name.
##########
sdks/go/pkg/beam/core/sdf/sdf.go:
##########
@@ -96,3 +98,12 @@ type WatermarkEstimator interface {
// splits or checkpoints to advance the output watermark of the restriction's stage.
CurrentWatermark() time.Time
}
+
+// ToWatermarkEstimator is an interface used to represent a user defined watermark estimator that
+// has the ability to observe timestamps of elements outputted from a ParDo's emit function.
+type ToWatermarkEstimator interface {
Review Comment:
If TO is supposed to be an acronym for TimestampObserving, please capitalize it accordingly. As it stands, it sounds like a type that converts types to WatermarkEstimators.
I recommend simplifying it to `TimestampObserver`. Yes ,it only applies to WatermarkEstimators with the embed, but that can also be clarified in the type documentation, rather than the name.
--
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