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

classic Classic list List threaded Threaded
21 messages Options
12
Reply | Threaded
Open this post in threaded view
|

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

felixzheng
Thanks very much Yang Wang.

Cheers,
Canbin Zheng

Yang Wang <[hidden email]> 于2020年2月27日周四 下午9:19写道:

> Great work! I could help to review and test.
>
> Best,
> Yang
>
> Canbin Zheng <[hidden email]> 于2020年2月27日周四 下午4:24写道:
>
>> Hi, everyone,
>>
>> I have pushed a PR <https://github.com/apache/flink/pull/11233> for this
>> issue, looking forward to your feedback.
>>
>>
>> Cheers,
>> Canbin Zheng
>>
>> Canbin Zheng <[hidden email]> 于2020年2月26日周三 上午10:39写道:
>>
>>> Thanks for the detailed PR advice, I would separate the commits as clear
>>> as possible to help the code reviewing.
>>>
>>>
>>> Cheers,
>>> Canbin Zheng
>>>
>>> tison <[hidden email]> 于2020年2月25日周二 下午10:11写道:
>>>
>>>> Thanks for your clarification Yang! We're on the same page.
>>>>
>>>> Best,
>>>> tison.
>>>>
>>>>
>>>> Yang Wang <[hidden email]> 于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
>>>>> commits/PRs to make the review easier. The reviewers may need some
>>>>> context
>>>>> to get the point.
>>>>>
>>>>>
>>>>>
>>>>> Best,
>>>>> Yang
>>>>>
>>>>> tison <[hidden email]> 于2020年2月25日周二 下午8:23写道:
>>>>>
>>>>>> 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 abstraction or extracting
>>>>>> a clumsy
>>>>>> "common" interface doesn't benefit.
>>>>>>
>>>>>> Best,
>>>>>> tison.
>>>>>>
>>>>>> [1]
>>>>>> https://github.com/apache/flink/commit/1f2969357c441e24b71daef83d21563da9a93bb4
>>>>>> [2] https://github.com/apache/flink/pull/9832
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> tison <[hidden email]> 于2020年2月25日周二 下午8:08写道:
>>>>>>
>>>>>>> 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 version of decorators are not api compatible and
>>>>>>> there is no reason we keep both
>>>>>>> of them.
>>>>>>>
>>>>>>> Best,
>>>>>>> tison.
>>>>>>>
>>>>>>>
>>>>>>> Yang Wang <[hidden email]> 于2020年2月25日周二 下午7:50写道:
>>>>>>>
>>>>>>>> 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 decorators in the kube
>>>>>>>> client implementation.
>>>>>>>>
>>>>>>>>
>>>>>>>> Best,
>>>>>>>> Yang
>>>>>>>>
>>>>>>>> Canbin Zheng <[hidden email]> 于2020年2月25日周二 下午6:16写道:
>>>>>>>>
>>>>>>>>> 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 refactoring PR. Actually I won't insist on one PR, could it be
>>>>>>>>> possible that I first try out with one PR and let the committers help
>>>>>>>>> assess whether it is necessary to split the changes into several PRs?
>>>>>>>>> Kindly expect to see your reply.
>>>>>>>>>
>>>>>>>>
12