[GitHub] [flink] TisonKun commented on issue #9643: [FLINK-13961][client] Remove obsolete abstraction JobExecutor(Service)
TisonKun commented on issue #9643: [FLINK-13961][client] Remove obsolete abstraction JobExecutor(Service) URL: https://github.com/apache/flink/pull/9643#issuecomment-529840825 Now I agree with that we don't have to rush into the removal since there are still users of this interface. I will revisit this topic when the prerequisite in my mind ready. Closed pull request and closed the corresponding JIRA as not a problem. Thanks for your review @tillrohrmann ! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] TisonKun commented on issue #9643: [FLINK-13961][client] Remove obsolete abstraction JobExecutor(Service)
TisonKun commented on issue #9643: [FLINK-13961][client] Remove obsolete abstraction JobExecutor(Service) URL: https://github.com/apache/flink/pull/9643#issuecomment-529727140 @flinkbot run travis This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] TisonKun commented on issue #9643: [FLINK-13961][client] Remove obsolete abstraction JobExecutor(Service)
TisonKun commented on issue #9643: [FLINK-13961][client] Remove obsolete abstraction JobExecutor(Service) URL: https://github.com/apache/flink/pull/9643#issuecomment-529726901 Hi @tillrohrmann, thanks for your review. I don't think this is an issue with high priority, but let me share you some of my concerns. - `JobExecutor` and `JobExecutorServices` provide poor abstraction based on our codebase. `JobExecutor` has only one method `executeJobBlocking(JobGraph)`. In my opinion, in order to submit a job and request the result we are supposed to use the client instead of directly access the cluster. `JobExecutorServices` is introduced for the usage in `LocalExecutor#executePlan(Plan)`. - A real demand will require clear abstraction. Back to the introduction of `JobExecutor` and `JobExecutorService`, we are required to provide an abstraction of MiniCluster and FlinkMiniCluster, which clearly points to the interface `executeJobBlocking(JobGraph)`. Now and in the near future, we don't have another `JobExecutor`. Anytime when we find another class takes similar responsibility like `MiniCluster`, we can always add an abstraction bases on that similarity. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] TisonKun commented on issue #9643: [FLINK-13961][client] Remove obsolete abstraction JobExecutor(Service)
TisonKun commented on issue #9643: [FLINK-13961][client] Remove obsolete abstraction JobExecutor(Service) URL: https://github.com/apache/flink/pull/9643#issuecomment-529364151 travis fails on NOTICE-binary checker. I don't think it is relevant to this pull request. Instead, it looks like an instance of [FLINK-14009](https://issues.apache.org/jira/browse/FLINK-14009). @tillrohrmann is it relevant? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
