Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-27 Thread Canbin Zheng
Thanks very much Yang Wang. Cheers, Canbin Zheng Yang Wang 于2020年2月27日周四 下午9:19写道: > Great work! I could help to review and test. > > Best, > Yang > > Canbin Zheng 于2020年2月27日周四 下午4:24写道: > >> Hi, everyone, >> >> I have pushed a PR for this >>

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-27 Thread Yang Wang
Great work! I could help to review and test. Best, Yang Canbin Zheng 于2020年2月27日周四 下午4:24写道: > Hi, everyone, > > I have pushed a PR for this > issue, looking forward to your feedback. > > > Cheers, > Canbin Zheng > > Canbin Zheng 于2020年2月26日周三

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-27 Thread Canbin Zheng
Hi, everyone, I have pushed a PR for this issue, looking forward to your feedback. Cheers, Canbin Zheng Canbin Zheng 于2020年2月26日周三 上午10:39写道: > Thanks for the detailed PR advice, I would separate the commits as clear > as possible to help the code

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-25 Thread Canbin Zheng
Thanks for the detailed PR advice, I would separate the commits as clear as possible to help the code reviewing. Cheers, Canbin Zheng tison 于2020年2月25日周二 下午10:11写道: > Thanks for your clarification Yang! We're on the same page. > > Best, > tison. > > > Yang Wang 于2020年2月25日周二 下午10:07写道: > >>

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-25 Thread tison
Thanks for your clarification Yang! We're on the same page. Best, tison. Yang Wang 于2020年2月25日周二 下午10:07写道: > Hi tison, > > I do not mean to keep two decorator at the same. Since the two decorators > are > not api compatible, it is meaningless. I am just thinking how to organize > the >

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-25 Thread Yang Wang
Hi tison, I do not mean to keep two decorator at the same. Since the two decorators are not api compatible, it is meaningless. I am just thinking how to organize the commits/PRs to make the review easier. The reviewers may need some context to get the point. Best, Yang tison 于2020年2月25日周二

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-25 Thread tison
The process in my mind is somehow like this commit[1] which belongs to this pr[2] that we firstly introduce the new implementation and then replace it with the original one. The difference is that these two versions of decorators are not api compatible while adding a switch for such an internal

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-25 Thread tison
I agree for separating commits we can have multiple commits that firstly add the new parameters and decorators, and later replace current decorators with new decorators which are well unit tested. However, it makes no sense we have two codepaths from FlinkKubeClient to decorators since these two

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-25 Thread Yang Wang
I think if we could, splitting into as many PRs as possible is good. Maybe we could introduce the new designed decorators and parameter parser first, and leave the existing decorators as legacy. Once all the new decorators is ready and well tested, we could remove the legacy codes and use the new

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-25 Thread Canbin Zheng
Hi, Till, Great thanks for your advice, I totally agree with you to split the changes up in as many PRs as possible. The part of "Parameter Parser" is trivial so that we prefer to make one PR to avoid adapting a lot of pieces of code that would be deleted immediately with the following decorator

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-25 Thread Till Rohrmann
Just a quick meta comment concerning one PR vs. two PRs vs. multiple PRs: I would recommend to split the changes up in as many PRs as possible. In my experience, a large uber PR usually won't be reviewed as thoroughly because of the sheer size. Moreover, the reviewing usually takes much longer. Of

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-24 Thread Canbin Zheng
Thanks, Yang Wang and tison, Since we have reached a consensus on the decorator design evolution and the parameters parser, and after an offline discussion with tison and Yang Wang, we have agreed on attaching one PR instead of two to avoid repeated code adapting and reviewing. Thanks for all

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-24 Thread tison
Thank Canbin for starting the discussion and thanks for participating so far. I agree that we can start with the "Parameters Parser" part and actually split it into a self-contained issue. Best, tison. Yang Wang 于2020年2月24日周一 下午8:38写道: > It seems that we could benefit a lot from the

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-24 Thread Yang Wang
It seems that we could benefit a lot from the "Parameters Parser". Let's start to add the dedicated jobmanager/taskmanager config parser classes. Best, Yang Canbin Zheng 于2020年2月24日周一 上午10:39写道: > Hi, Yang Wang, > > Thanks for the feedback. > > > Parameters Parser > > I can think of many

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-23 Thread Canbin Zheng
Hi, Yang Wang, Thanks for the feedback. > Parameters Parser I can think of many benefits of the dedicated parameter parsing/verifying tools, here is some of them: 1. Reuse the parameters parsing and verifying code. 2. Ensure consistent handling logic for the same setting. 3. Simplify some of

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-23 Thread Yang Wang
> The new introduced decorator After some offline discussion with Canbin and tison, i totally understand the evolved decorator design. Each decorator will be self-contained and is responsible for just one thing. Currently, if we want to mount a new config file to jobmanager/taskmanager, then both

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-21 Thread felixzheng zheng
Great thanks for the quick feedback Till. You are right; it is not a fundamentally different approach compared to what we have right now, all the Kubernetes resources created are the same, we aim to evolve the existing decorator approach so that, 1. the decorators are monadic and smaller in size

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-21 Thread Till Rohrmann
Thanks for starting this discussion Canbin. If I understand your proposal correctly, then you would like to evolve the existing decorator approach so that decorators are monadic and smaller in size and functionality. The latter aspect will allow to reuse them between the client and the cluster.

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-21 Thread felixzheng zheng
Thanks for the feedback @Yang Wang. I would like to discuss some of the details in depth about why I am confused about the existing design. Question 1: How do we mount a configuration file? For the existing design, 1. We need several classes to finish it: 1.

Re: [DISCUSS] FLINK-16194: Refactor the Kubernetes architecture design

2020-02-20 Thread Yang Wang
Hi Canbing, Thanks a lot for sharing your thoughts to improve the Flink on K8s native integration. Frankly speaking, your discussion title confuses me and i am wondering whether you want to refactor the whole design. However, after i dive into the details and the provided document, i realize