Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BEAM-4643] Allow to check early panes of a window #5811

Merged
merged 4 commits into from
Oct 9, 2018
Merged

[BEAM-4643] Allow to check early panes of a window #5811

merged 4 commits into from
Oct 9, 2018

Conversation

lhauspie
Copy link
Contributor

Just adding the ability to check the early panes of a specific window (not only the global window).

Just by adding inEarlyPanes(BoundedWindow window) method in PAssert and its implementations, we are now capable of doing:

PAssert.that(teamScores)
    .inEarlyPanes(intervalWindow(05, 20))
        .containsInAnyOrder(
            KV.of("black", 1),  // First item (Black, 1)
            KV.of("black", 2))  // Second item (Black, 1) cumulated with the first one
    .inOnTimePane(intervalWindow(05, 20))
        .containsInAnyOrder(
            KV.of("black", 2))  // No new items since the last early element
    .inFinalPane(intervalWindow(05, 20))
        .containsInAnyOrder(
            KV.of("black", 10)); // New late data (black, 8)

NB: intervalWindow(05, 20) returns an new IntervalWindow from 00:05:00 to 00:20:00

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Spark
Go Build Status --- --- --- --- ---
Java Build Status Build Status Build Status Build Status Build Status Build Status
Python Build Status --- Build Status
Build Status
--- --- ---

@kennknowles
Copy link
Member

We have turned on autoformatting of the codebase, which causes small conflicts across the board. You can probably safely rebase and just keep your changes. Like this:

$ git rebase
... see some conflicts
$ git diff
... confirmed that the conflicts are just autoformatting
... so we can just keep our changes are do our own autoformat
$ git checkout --theirs --
$ git add -u
$ git rebase --continue
$ ./gradlew spotlessJavaApply

Please ping me if you run into any difficulty.

@lhauspie
Copy link
Contributor Author

retest this please

@lhauspie
Copy link
Contributor Author

lhauspie commented Jun 29, 2018

I don't understand what's going wrong with the Jenkins Build.

On local, I've got the following outputs (BUILD SUCCESSFUL):

$ ./gradlew :beam-sdks-java-io-hadoop-input-format:test
    [...]
    > Task :beam-sdks-java-io-hadoop-input-format:testClasses
    
    > Task :beam-sdks-java-io-hadoop-input-format:test
    Java HotSpot(TM) Server VM warning: You have loaded library /tmp/libnetty-transport-native-epoll7569020236429498545.so which might have disabled stack guard. The VM will try to fix the stack guard now.
    It's highly recommended that you fix the library with 'execstack -c <libfile>', or link it with '-z noexecstack'.
    
    Deprecated Gradle features were used in this build, making it incompatible with Gradle 5.0.
    See https://docs.gradle.org/4.8/userguide/command_line_interface.html#sec:command_line_warnings
    
    BUILD SUCCESSFUL in 2m 3s
    53 actionable tasks: 41 executed, 12 up-to-date
$ ./gradlew :beam-sdks-java-io-elasticsearch-tests-2:test
    [...]
    > Task :beam-sdks-java-io-elasticsearch-tests-2:testClasses
    > Task :beam-sdks-java-io-elasticsearch-tests-2:test
    
    Deprecated Gradle features were used in this build, making it incompatible with Gradle 5.0.
    See https://docs.gradle.org/4.8/userguide/command_line_interface.html#sec:command_line_warnings
    
    BUILD SUCCESSFUL in 25s
    48 actionable tasks: 6 executed, 42 up-to-date

@kennknowles, could you please tell me if I introduced some bugs or did something wrong ?

@kennknowles
Copy link
Member

I'm afraid I need to find another reviewere here. @reuvenlax can you take a look or pass it on?

@apilloud
Copy link
Member

run java precommit

@apilloud
Copy link
Member

ping @reuvenlax

@lhauspie
Copy link
Contributor Author

lhauspie commented Oct 8, 2018

retest this please

@lhauspie
Copy link
Contributor Author

lhauspie commented Oct 8, 2018

@lukecwik, does this PR need some actions from me or something else to be merged ?

Thanks in advance ;)

@akedin
Copy link
Contributor

akedin commented Oct 9, 2018

looking

@akedin
Copy link
Contributor

akedin commented Oct 9, 2018

retest this please

1 similar comment
@akedin
Copy link
Contributor

akedin commented Oct 9, 2018

retest this please

@akedin akedin merged commit c356c57 into apache:master Oct 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants