You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/03/25 19:20:41 UTC

[GitHub] [beam] reuvenlax opened a new pull request #11226: [BEAM-9557] Fix timer window boundary checking

reuvenlax opened a new pull request #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226
 
 
   R: @steveniemitz 

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

[GitHub] [beam] reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-605685523
 
 
   @aaltay Has part of this change already been merged to master? I notice that SimpleDoFnRunner.java at head already contains these changes, but the unit tests are not it.

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

[GitHub] [beam] steveniemitz commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
steveniemitz commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-605685701
 
 
   it looks like #11252 was merged instead of this one.

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

[GitHub] [beam] amaliujia edited a comment on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
amaliujia edited a comment on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-604620053
 
 
   This failed test might be relevant to this PR: org.apache.beam.sdk.transforms.ParDoTest$TimerTests.testOutOfBoundsEventTimeTimer
   
   (link https://builds.apache.org/job/beam_PreCommit_Java_Phrase/1930/)

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

[GitHub] [beam] aaltay commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
aaltay commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-605098171
 
 
   > @amaliujia appears that test looks for specific strings in the exception (always a recipe for a brittle test). I changed "event time timer" to "event-time timer" which broke that test. Will fix.
   
   Agreed, it is brittle test.
   
   @reuvenlax are you planning to fix the test, or could @amaliujia send a fixup commit here to change "event-time" to "event time"? This is only release blocking issue currently, I would like to unblock it sooner.

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

[GitHub] [beam] reuvenlax merged pull request #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
reuvenlax merged pull request #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226
 
 
   

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

[GitHub] [beam] amaliujia commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
amaliujia commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-610078277
 
 
   LGTM

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

[GitHub] [beam] amaliujia commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
amaliujia commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-610078537
 
 
   @reuvenlax 
   
   do you need help on the failed java tests?

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

[GitHub] [beam] reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-605693639
 
 
   I see. We should still merge this so we get the unit-test coverage.
   
   On Sun, Mar 29, 2020 at 12:19 PM Steven Niemitz <no...@github.com>
   wrote:
   
   > it looks like #11252 <https://github.com/apache/beam/pull/11252> was
   > merged instead of this one.
   >
   > —
   > You are receiving this because you were mentioned.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/beam/pull/11226#issuecomment-605685701>, or
   > unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AFAYJVMT6FWXML2TDA2X4JLRJ6NKRANCNFSM4LTWF4CQ>
   > .
   >
   

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

[GitHub] [beam] steveniemitz commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
steveniemitz commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-605334008
 
 
   looks great!  These tests will be really helpful to ensure we don't have more regressions in the future.  Thanks!

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

[GitHub] [beam] amaliujia commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
amaliujia commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-604211801
 
 
   Run Java PreCommit

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

[GitHub] [beam] reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-605313456
 
 
   @aaltay fixing this now

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

[GitHub] [beam] reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-605324853
 
 
   retest this please

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

[GitHub] [beam] steveniemitz commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
steveniemitz commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-604042702
 
 
   can we add a unit test to validate this behavior as well?  

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

[GitHub] [beam] reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-605324797
 
 
   retest this please

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

[GitHub] [beam] reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-605332723
 
 
   @steveniemitz tests added

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

[GitHub] [beam] aaltay commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
aaltay commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-605323118
 
 
   > @aaltay added new unit tests and fixed the broken one. However I'm noticing that test triggering seems broken on a number of PRs now. Is something wrong with the Jenkins master?
   
   Yes. Ongoing issue. @yifanzou is working on it. Repeating the test triggers a few times usually helps.

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

[GitHub] [beam] reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-605321717
 
 
   @aaltay added new unit tests and fixed the broken one. However I'm noticing that test triggering seems broken on a number of PRs now. Is something wrong with the Jenkins master?

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

[GitHub] [beam] amaliujia commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
amaliujia commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-604620053
 
 
   This failed test might be relevant to this PR: org.apache.beam.sdk.transforms.ParDoTest$TimerTests.testOutOfBoundsEventTimeTimer

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

[GitHub] [beam] reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-604729044
 
 
   @amaliujia appears that test looks for specific strings in the exception (always a recipe for a brittle test). I changed "event time timer" to "event-time timer" which broke that test. Will fix.

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

[GitHub] [beam] reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-605324746
 
 
   retest this please

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

[GitHub] [beam] reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking

Posted by GitBox <gi...@apache.org>.
reuvenlax commented on issue #11226: [BEAM-9557] Fix timer window boundary checking
URL: https://github.com/apache/beam/pull/11226#issuecomment-605383329
 
 
   Run Java PreCommit

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