Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/750#issuecomment-184165224
This looks pretty good by now. I have a few remaining comments, otherwise
this looks good to go:
1. There is a duplicate and incorrect test dependency in `flink-tests`
(critical, will break the build later)
2. The are two types of Exceptions:
- ProgramStopException
- StoppingException
Both simply extend `Exception`. Can we consolidate these into one
class?
3. The JobManager tries to stop the job in state `CREATED`, `RUNNING`,
and `RESTARTING`. Are we sure that the states `CREATED` and `RESTARTING` behave
well?
4. Minor comment: Adding the generic `SRC` parameter to the
`SourceStreamTask` looks like a bit of overkill for only adding the
`StoppableFunction` interface to the generic function type. A simple cast in
`StoppableSourceStreamTask.stop()` would probably be the more lightweight
change.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
