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

[FLINK-3523] [Storm-Compatibility] Added SplitStreamMapper to program… #1844

Closed
wants to merge 3 commits into from

Conversation

mjsax
Copy link
Member

@mjsax mjsax commented Mar 31, 2016

… to get rid of SplitStreamType wrapper

@mxm
Copy link
Contributor

mxm commented Apr 4, 2016

LGTM. Perhaps we could add a test case for this example?

@mjsax
Copy link
Member Author

mjsax commented Apr 4, 2016

This is an additional example that runs forever -- it was never designed for getting tested. Furthermore, there are already two test for split-stream functionality.

@mxm
Copy link
Contributor

mxm commented Apr 4, 2016

IMHO we should have a way to check that examples are working properly. We don't have to address this in this PR though.

@mjsax
Copy link
Member Author

mjsax commented Apr 4, 2016

Change example slightly (make it run finite and added a check for even/odd stream) and added IT-Case.

@StephanEwen
Copy link
Contributor

For the test, two comments:

  • Can you add it to another ITCase (saves bringing an extra mini cluster)
  • Also, it will probably not run on windows. Writing to a temp file and deleting that should make this compatible.

added tmp files
@mjsax
Copy link
Member Author

mjsax commented Apr 5, 2016

Done. I needed to restructure class hierarchy, because StreamingMultipleProgramsTestBase did not provided methods for tmp-file handling...

private static final long serialVersionUID = 5213888269197438892L;
private final Tuple2<String, Integer> out;
private final boolean evenOrOdd; // true: even -- false: odd
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about isEven as name here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Can change it.

@mjsax
Copy link
Member Author

mjsax commented Apr 6, 2016

Test fail on Java8 module... Can I merge this @mxm @StephanEwen?

@mxm
Copy link
Contributor

mxm commented Apr 6, 2016

+1 to merge

@asfgit asfgit closed this in 02ef68f Apr 6, 2016
@mjsax mjsax deleted the flink-3523-split-example branch April 6, 2016 14:47
fijolekProjects pushed a commit to fijolekProjects/flink that referenced this pull request May 1, 2016
… to get rid of SplitStreamType wrapper

 - additionally reworked split ITCases
   * unified to single test
   * make Windows compatible (using tmp files)
   * added test case for embedded splitting

This closes apache#1844
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants