[FLINK-7762, FLINK-8167] Clean up and harden WikipediaEditsSource #5102
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Minor eager sanity checks - Use UUID suffix for nickname. As reported in FLINK-8167, the current nickname suffix can result in nickname clashes which lead to test failures.
…er test The WikipediaEditsSourceTest unnecessarily implements an integration test that starts a FlinkMiniCluster and executes a small Flink program. This simply creates a source and executes run in a separate thread until a single WikipediaEditEvent is received.
|
The changes look good! But you comment about needing that IRC channel seem valid. If we don't have the test, however, we would have no way of knowing that the code at least works. (We could also drop the source completely) |
|
I would be in favour of removing this or moving it to Bahir, but it is currently used in a doc example. Besides that I doubt that this is of much value to users. If you think it's OK with it, let's merge this for now and think about the example thing really needs it. |
|
LGTM. +1 to merge this now, and we think about removing the source as well as reworking the doc example to not use it (I think it might be nice to directly kick off the docs with an intro example that read / writes to Kafka), as a separate issue. |
|
merging. |
asfgit
pushed a commit
that referenced
this pull request
Dec 4, 2017
…er test The WikipediaEditsSourceTest unnecessarily implements an integration test that starts a FlinkMiniCluster and executes a small Flink program. This simply creates a source and executes run in a separate thread until a single WikipediaEditEvent is received. This closes #5102.
glaksh100
pushed a commit
to lyft/flink
that referenced
this pull request
Jun 6, 2018
…er test The WikipediaEditsSourceTest unnecessarily implements an integration test that starts a FlinkMiniCluster and executes a small Flink program. This simply creates a source and executes run in a separate thread until a single WikipediaEditEvent is received. This closes apache#5102.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the purpose of the change
This pull requests addresses two related issues with the WikipediaEditsSource. It makes the WikipediaEditsSourceTest a proper test instead of unnecessarily starting a FlinkMiniCluster and addresses a potential test instability.
In general, the WikipediaEditsSource is not in good shape and could benefit from further refactoring. One potential area for improvement is integration with the asynchronous channel listener that reports events like errors or being kicked out of a channel, etc.
I did not do this due to time constraints and the fact that this is not a production source. In general, it is questionable whether we should keep the test as is or remove it since it depends on connectivity to an IRC channel.
Brief change log
Verifying this change
This change is a rework/code cleanup without any new test coverage.
Does this pull request potentially affect one of the following parts:
flink-test-utils-junit@Public(Evolving): noDocumentation