boyuanzz commented on a change in pull request #13421:
URL: https://github.com/apache/beam/pull/13421#discussion_r538893012
##########
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:
Thanks for the explanation! It sounds more like an API decision since
implementation details are mostly the same.
My preference is:
```python
class TimerDoFn(beam.DoFn):
timer_spec = userstate.TimerSpec('timer',
userstate.TimeDomain.WATERMARK)
timer_family_spec = userstate.TimerFamilySpec('timer',
userstate.TimeDomain.WATERMARK)
def process(self,
element,
timer=beam.DoFn.TimerParam(timer_spec),
timer_map=beam.DoFn.TimerParam(timer_family_spec)):
@userstate.on_timer(timer_spec)
def process_timer(
self,
ts=beam.DoFn.TimestampParam,
timer=beam.DoFn.TimerParam(timer_spec)):
@userstate.on_timer(timer_family_spec)
def process_timer(
self,
ts=beam.DoFn.TimestampParam,
timer=beam.DoFn.TimerParam(timer_family_spec)):
```
And you are proposing the API like:
```python
class TimerDoFn(beam.DoFn):
timer_spec = userstate.TimerSpec('timer',
userstate.TimeDomain.WATERMARK)
timer_family_spec = userstate.TimerSpec('timer',
userstate.TimeDomain.WATERMARK)
def process(self,
element,
timer=beam.DoFn.TimerParam(timer_spec),
timer_map=beam.DoFn.TimerFamilyParam(timer_family_spec)):
@userstate.on_timer(timer_spec)
def process_timer(
self,
ts=beam.DoFn.TimestampParam,
timer=beam.DoFn.TimerParam(timer_spec)):
@userstate.on_timer(timer_family_spec)
def process_timer(
self,
ts=beam.DoFn.TimestampParam,
timer=beam.DoFn.TimerFamilyParam(timer_family_spec)):
```
And within your preferred API, the suer is not allowed to use the same name
for both specs unless you want to additional logic around timer param to add
prefix or just throw validation errors.
Do you think it's a good idea to ask options on dev list?
----------------------------------------------------------------
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]
