boyuanzz commented on a change in pull request #13421:
URL: https://github.com/apache/beam/pull/13421#discussion_r538816533
##########
File path: sdks/python/apache_beam/transforms/userstate.py
##########
@@ -184,29 +186,6 @@ def to_runner_api(self, context, key_coder, window_coder):
coders._TimerCoder(key_coder, window_coder)))
-# TODO(BEAM-9602): Provide support for dynamic timer.
-class TimerFamilySpec(object):
Review comment:
If one DoFn defines one `TimerSpec` and uses it in both `TimerParam` and
`TimerFamilyParam`, we will have collisions on this spec, right? And in this
case, there is no way to register one callback for timer `TimerSpec and one
callback for timer family `TimerSpec` since we are using maps in in
implemetation.
I'm leaning to keep `TimerFamilySpec` and use `TimerParam` for both
`TimerSpec` and `TimerFamilySpec`, where the `prefix` field will help us reduce
the collision.
##########
File path: sdks/python/apache_beam/runners/direct/direct_userstate.py
##########
@@ -225,14 +225,28 @@ def __init__(self, step_context, dofn, key_coder):
self.cached_states = {}
self.cached_timers = {}
-
- def get_timer(self, timer_spec, key, window, timestamp, pane):
+ self.cached_timer_families = {}
+
+ def get_timer(
+ self,
+ timer_spec: userstate.TimerSpec,
+ key,
+ window,
+ timestamp,
+ pane,
+ is_timer_family=False
Review comment:
Would it be better to have separated functions like get_timer(),
get_timer_family instead of using a boolean
----------------------------------------------------------------
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:
[email protected]
