beam-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [beam] acrites commented on issue #10988: [BEAM-9382] Clean up of TestStreamTranscriptTests.
Date Sun, 08 Mar 2020 22:58:56 GMT
acrites commented on issue #10988: [BEAM-9382] Clean up of TestStreamTranscriptTests.
URL: https://github.com/apache/beam/pull/10988#issuecomment-596263133
 
 
   RE fixing the Python direct runner: right now we just use the return value
   from on_fire for each trigger to set the final bit. This doesn't take into
   account allowed lateness. So we'd probably need to update all of the
   triggers.
   
   Maybe just a single test for now that is disabled and has the final bit
   check? Or did you want to duplicate all the tests? I'm thinking something
   like this one:
   
   name: fixed_allowed_lateness_final_set
   broken_on:
     - SwitchingDirectRunner  # Doesn't set final field properly with late
   data.
   window_fn: FixedWindows(10)
   trigger_fn: AfterWatermark(early=AfterCount(2), late=AfterCount(1))
   timestamp_combiner: OUTPUT_AT_EOW
   allowed_lateness: 10
   transcript:
     - input: [1, 2]
     - expect:
         - {window: [0, 9], values: [1, 2], timestamp: 9, final: false}
     - input: [3]
     - watermark: 100
     - expect:
         - {window: [0, 9], values: [1, 2, 3], timestamp: 9, final: true}
   
   
   
   On Fri, Mar 6, 2020 at 11:45 AM Robert Bradshaw <notifications@github.com>
   wrote:
   
   > The Python direct runner does support PaneInfo, but it doesn't (yet)
   > support allowed lateness, so the "final" bit is wrong. Yes, we should fix
   > this, though hopefully on the FnApiRunner once it supports streaming rather
   > than invest too much into the old direct runner (unless it's cheap to do).
   >
   > I would rather disable the "final" bit check for these tests (and add a
   > new, disabled test that checks this bit) than disable these tests entirely
   > for the direct runner. So as is, this change LGTM.
   >
   > We could also consider expanding the API to indicate certain features are
   > broken on certain runners, rather than entire tests, which would allow us
   > to fully specify the expected results and still produce partial validation
   > on incomplete/in-progress runners.
   >
   > —
   > You are receiving this because you authored the thread.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/beam/pull/10988?email_source=notifications&email_token=ACOKCJXEORU3BA5EY2IWAFTRGFHD7A5CNFSM4K5AFZM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOCS4KY#issuecomment-595930667>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/ACOKCJR3YOU4QZIDUJOT7DLRGFHD7ANCNFSM4K5AFZMQ>
   > .
   >
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message