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