Thanks for the explanation! That makes sense. We may also want to update pydoc to state the usage explicitly since it's quite different from how Java SDK does it.
On Thu, Dec 10, 2020 at 11:13 AM Robert Bradshaw <[email protected]> wrote: > On Thu, Dec 10, 2020 at 11:05 AM Boyuan Zhang <[email protected]> wrote: > >> We should also consider whether the default used in set() should be the >>> empty string, or some value completely disjoint from any other string. (I'd >>> lean towards the latter.) >> >> From API layer, the default value could be None for non-dynamic timer. >> When translating to timer data message, None will be translated into empty >> string. >> > > Yes, but to keep them disjoint we'd want to translate the empty string > into something else. > > >> It seems like either timer.set(timestamp, dynamic_timer_tag=a_tag) or >> set_dynamic means we will allow users to use one TimerSpec instance for >> both static timer and dynamic timer. >> > > Sure. > > >> I'm wondering for registering on_timer callback, are we going to allow >> users to register 2 callbacks(one for dynamic timer, one for static timer) >> for single one TimerSpec instance, or just having one callback with >> DynamicTagParam, where when DynamicTagParam is None means current firing >> timer is a static one(otherwise, it's a dynamic one). >> > > No, there'll still be a single callback. You can inspect the tag if you > want. > > The view is that there aren't two types of timers--"static" timers are > just dynamic timers with a fixed (aka static) tag (that we provide for you > as a convenience). > > On Thu, Dec 10, 2020 at 10:43 AM Robert Bradshaw <[email protected]> >> wrote: >> >>> Yep. >>> >>> A slight variant on this is to add separate set_dynamic and >>> clear_dynamic methods, rather than letting set and clear take an optional >>> argument, but I'm not sure I like that as much as the simple extension you >>> proposed. >>> >>> We should also consider whether the default used in set() should be the >>> empty string, or some value completely disjoint from any other string. (I'd >>> lean towards the latter.) >>> >>> On Thu, Dec 10, 2020 at 10:23 AM Yichi Zhang <[email protected]> wrote: >>> >>>> Fair enough, I think we can also just extend existing timer API to >>>> allow setting a dynamic timer tag field: >>>> >>>> timer.set(timestamp) -> timer.set(timestamp, dynamic_timer_tag=a_tag) >>>> timer.clear() -> timer.clear(dynamic_timer_tag=a_tag) >>>> >>>> and have the default value of dynamic_timer_tag to be empty (the >>>> special case) >>>> >>>> On Wed, Dec 9, 2020 at 5:12 PM Robert Bradshaw <[email protected]> >>>> wrote: >>>> >>>>> On Wed, Dec 9, 2020 at 3:48 PM Kyle Weaver <[email protected]> >>>>> wrote: >>>>> >>>>>> Possibly a dumb question, but: if "the static timer is just a special >>>>>> case of the dynamic timer," why do we need to use different classes at >>>>>> all? >>>>>> >>>>> >>>>> I agree, I would argue that there is little if any value to the user >>>>> to distinguish between these two "types" of timers at all. >>>>> >>>>> >>>>>> On Wed, Dec 9, 2020 at 2:30 PM Yichi Zhang <[email protected]> wrote: >>>>>> >>>>>>> Hi, Beam community, >>>>>>> >>>>>>> I'm trying to add the dynamic timer >>>>>>> <https://issues.apache.org/jira/browse/BEAM-6857> support to the >>>>>>> python sdk. In java sdk, a dynamic timer is specified through declaring >>>>>>> a >>>>>>> TimerSpec of timerMap type and annotating it with @TimerFamily in >>>>>>> process method parameter: >>>>>>> >>>>>>> @TimerFamily("timers") private final TimerSpec timer = >>>>>>> TimerSpecs.timerMap(TimeDomain.EVENT_TIME); >>>>>>> >>>>>>> @ProcessElement public void process( >>>>>>> @Element KV<String, ValueT> element, >>>>>>> @Timestamp Instant elementTs, >>>>>>> @TimerFamily("timers") TimerMap timers) { >>>>>>> timers.set(element.getValue().getActionType(), elementTs); >>>>>>> } >>>>>>> >>>>>>> >>>>>>> In python, I'm trying to figure out how we should differentiate a >>>>>>> dynamic timer to the conventional static timer, at the timer spec >>>>>>> declaration or process parameters annotation, or both: >>>>>>> >>>>>>> Approach 1: if we differentiate them only at timer spec declaration >>>>>>> it looks like: >>>>>>> >>>>>>> class TimerDoFn(beam.DoFn): >>>>>>> timer_spec = userstate.TimerSpec('static_timer', >>>>>>> userstate.TimeDomain.WATERMARK) >>>>>>> timer_family_spec = userstate.TimerFamilySpec('dynamic_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_map( >>>>>>> self, >>>>>>> ts=beam.DoFn.TimestampParam, >>>>>>> dynamic_tag=DoFn.DynamicTagParam, >>>>>>> timer_map=beam.DoFn.TimerParam(timer_family_spec)) >>>>>>> >>>>>>> Approach 2: if only at parameter annotation: >>>>>>> >>>>>>> class TimerDoFn(beam.DoFn): >>>>>>> timer_spec = userstate.TimerSpec('static_timer', >>>>>>> userstate.TimeDomain.WATERMARK) >>>>>>> timer_family_spec = userstate.TimerSpec('dynamic_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)): >>>>>>> >>>>>>> >>>>>>> Approach 3: we can differentiate them at both places, but since the >>>>>>> static timer is just a special case of the dynamic timer, it will >>>>>>> introduce >>>>>>> a certain amount of code duplication. >>>>>>> >>>>>>> I'm trying to decide which one offers the best API readability and >>>>>>> usability. >>>>>>> >>>>>>> Anyone have any opinions? >>>>>>> >>>>>>> Thanks, >>>>>>> Yichi >>>>>>> >>>>>>
