Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/742#issuecomment-107768981
Some important comments:
- Exceptions should always propagate, and exceptions during cancelling
can be thrown. The `Task` class filters out exceptions that come after the
transition to the "canceling" state. Don't try to be super smart there,
propagate your exceptions, and the context will decide whether they should be
logged.
- The RabbitMQ source is not doing any proper failure handling
(critical!). I opened a separate JIRA issue for that.
- Just commenting out the twitter sources is bad style, in my opinion.
There is actually no chance of getting this pull request in, and this one here
is a release blocker, while the Twitter Source one is not a release blocker.
- All functions set their `running` flag to true at the beginning of the
`run()` method. In the case of races between invoking and canceling the source,
it can mean that the flag is set to false in the cancel() method and then to
true in the run() method, resulting in a non-canceled source. I fixed some
cases in my follow up pull-request, but many are still in. Please make another
careful pass with respect to that.
- In general, the streaming sources are extremely badly commented. This
needs big improvements!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
