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 >>>>>> >>>>>
