All of your plans sound good. Private constructors, yes. Just one Trigger subclass, yes. I don't have strong feelings about the naming here. It could be just AfterWatermark or AfterWatermark.PastEndOfWindow.
The conversion from Trigger to TriggerStateMachine goes through beam_runner_api.proto which is already designed the right way, so I think you can just go ahead and as long as you can convert the Java Trigger to the proto-generated RunnerApi.Trigger (in runners-core-construction Triggers.java) it will continue to work. Getting the core SDK nice for the first stable release is most important and we can always tidy up runners/core-java and runners/core-constructors-java later (we are not going to guarantee any compatibility for those). The state machines could use a lot of cleanup, if that is what motivates you, of course. Kenn On Thu, Apr 20, 2017 at 10:53 PM, Wesley Tanaka <wtan...@yahoo.com.invalid> wrote: > Thanks for parsing the formatting on my original mail. :) > What else is required for that change that I didn't outline? I think > there's a state machine class in runners core that would need to be > deleted? Is there anything else like a registry of triggers that might > need to be edited? > Do you have a preference for the name of the final merged class? Would > you want to just make the top level class a Trigger subclass directly with > private constructors? > > On Thu, Apr 20, 2017 at 15:19, Kenneth Knowles<k...@google.com.INVALID> > wrote: Hi Wesley, > > I agree with getting rid of FromEndOfWindow and just having AfterWatermark. > I have wanted to do this for a while, so if someone else does it that would > be great. > > This comes from the separation of OnceTrigger (triggers that fire just > once) and Trigger (triggers that may fire multiple times). But this > separation is not that important and a little fake - if you use > FromEndOfWindow and nonzero allowed lateness you can get multiple panes due > to final panes. The more important distinction is actually between triggers > that finish and triggers that don't finish... > > Anyhow, even if we care about once vs not-once, we can just do analysis on > the constructed trigger instead of making the types and classes complex. > > Kenn > > > On Thu, Apr 20, 2017 at 12:59 AM, Wesley Tanaka <wtan...@yahoo.com.invalid > > > wrote: > > > AfterWatermarkEarlyAndLate has: > > public AfterWatermarkEarlyAndLate withEarlyFirings(OnceTrigger > > earlyTrigger) > > public AfterWatermarkEarlyAndLate withLateFirings(OnceTrigger > > lateTrigger) > > FromEndOfWindow has: > > public AfterWatermarkEarlyAndLate withEarlyFirings(OnceTrigger > > earlyFirings) public AfterWatermarkEarlyAndLate > > withLateFirings(OnceTrigger lateFirings) > > As a means of trying to understand the trigger API, I am curious if it > > might make sense conceptually to: > > 1. rename AfterWatermarkEarlyAndLate => EarlyAndLate since it is already > > an inner class of AfterWatermark2. get rid of FromEndOfWindow by having > > AfterWatermark.pastEndOfWindow() return a new > AfterWatermarkEarlyAndLate(Never.ever(), > > null); > > Or is there a value to having FromEndOfWindow be separate that I am not > > understanding? > > > > --- > > Wesley Tanaka > > https://wtanaka.com/ > >
