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-4798] CEPITCase.testSimpleKeyedPatternCEP test failure #2843

Closed
wants to merge 1 commit into from

Conversation

BorisOsipov
Copy link

I've reproduced this issue and found that it is a known issue.

- Use TestListResultSink instead of writing tests results on disk to avoid IO issues
- Move TestListResultSink from flink-tests_2.10 to flink-test-utils_2.10 to avoid IO issues unnecessary dependencies
@zentol
Copy link
Contributor

zentol commented Nov 25, 2016

Which "known issue" are you referring to?

@BorisOsipov
Copy link
Author

@@ -361,17 +351,18 @@ public String select(Map<String, Event> pattern) {
}
);

result.writeAsText(resultPath, FileSystem.WriteMode.OVERWRITE);
result.addSink(resultSink);
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 using DataStreamUtils#collect() here? Then we wouldn't need to move that sink class around.

Copy link
Author

Choose a reason for hiding this comment

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

I didn't find any of useDataStreamUtils#collect() in Flink tests.
DataStreamUtils#collect() places in <artifactId>flink-contrib</artifactId> module. Are you sure that is a good idea to depend on it?

I moved classes because I saw at least four implementations String\List sinks for testing purpose. I guess it's a good idea to migrate to one of them and get rid of usage temp files where possible.

What do you think? I'm newbie in a Flink and maybe wrong understand smth

Copy link
Contributor

Choose a reason for hiding this comment

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

wasn't aware that it sits in flink-contrib, nevermind my previous comment then.

@BorisOsipov BorisOsipov changed the title FLINK-4798] CEPITCase.testSimpleKeyedPatternCEP test failure [FLINK-4798] CEPITCase.testSimpleKeyedPatternCEP test failure Nov 29, 2016
@BorisOsipov
Copy link
Author

These changes not needed after FLINK-5332 fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants