Hi Junrui, Thanks for the clarification. On one hand, adding more methods into the RuntimeContext flat will increase the effort for users who will use RuntimeContext. But the impact is limited. It is fine. The big impact is, on the other hand, for users who want to focus on the execution config, they will need to find the needle in the haystack.
I just shared my thoughts and tried to help you look at the issue from many different angles and I am open to learning opinions from other contributors. Please feel free to proceed if there are no other objections. Best regards, Jing On Mon, Nov 20, 2023 at 6:50 AM Junrui Lee <jrlee....@gmail.com> wrote: > Hi Jing, > > Thank you for your feedback. I understand your concerns regarding putting > all methods into the RuntimeContext flat. > > I would like to share some of my thoughts on this matter. > Firstly, this FLIP only proposes the addition of three additional methods, > which should not impose too much extra burden on users. Secondly, I agree > that it is important to make it clearer for users to use the > RuntimeContext. However, reorganizing the RuntimeContext to achieve this > requires further discussion. We should focus on a more specific and unified > reorganization of the RuntimeContext interface in future work, rather than > implementing a temporary solution now. Therefore, I prefer not to add a > separate abstraction layer for these three methods in this FLIP. > > Please feel free to share any further thoughts. > > Best regards, > Junrui > > Jing Ge <j...@ververica.com> 于2023年11月20日周一 05:46写道： > >> Hi Junrui, >> >> Thanks for bringing this to our attention. First of all, it makes sense >> to deprecate RuntimeContext#getExecutionConfig. >> >> Afaic, this is an issue of how we design API with clean concepts/aspects. >> There are two issues mentioned in the FLIP: >> >> 1. short of user-facing abstraction - we just exposed ExecutionConfig >> which mixed methods for users with methods that should only be used >> internally. >> 2. mutable vs immutable - do we want users to be able to modify configs >> during job execution? >> >> An immutable user-facing abstraction design can solve both issues. All >> execution related configs are still consolidated into the abstraction class >> and easy to access. This is another design decision: flat vs. hierarchical. >> Current FLIP removed the execution config abstraction and put all methods >> into RuntimeContext flat, which will end up with more than 30 methods >> offered flat by the RuntimeContext. I am not sure if this could help users >> find the right method in the context of execution config better than >> before. >> >> I might miss something and look forward to your thoughts. Thanks! >> >> Best regards, >> Jing >> >> On Sat, Nov 18, 2023 at 11:21 AM Junrui Lee <jrlee....@gmail.com> wrote: >> >>> Hello Wencong, >>> >>> Thank you for your valuable feedback and suggestions. I want to clarify >>> that reviewing existing methods in the ExecutionConfig is not directly >>> related to the proposal in this FLIP. The main focus of this FLIP is to >>> deprecate the specific method RuntimeContext#getExecutionConfig(). I >>> believe it is important to keep the scope of this FLIP limited. However, >>> your suggestion can certainly be considered as a separate FLIP in the >>> future. >>> >>> Best regards, >>> Junrui >>> >>> Wencong Liu <liuwencle...@163.com> 于2023年11月17日周五 22:08写道： >>> >>>> Hello Junrui, >>>> >>>> >>>> Thanks for the effort. I agree with the proposal to deprecate the >>>> getExecutionConfig() method in the RuntimeContext class. Exposing >>>> the complex ExecutionConfig to user-defined functions can lead to >>>> unnecessary complexity and risks. >>>> >>>> >>>> I also have a suggestion. We could consider reviewing the existing >>>> methods in ExecutionConfig. If there are methods that are defined >>>> in ExecutionConfig but currently have no callers, we could consider >>>> annotating them as @Internal or directly removing them. Since >>>> users are no longer able to access and invoke these methods, >>>> it would be beneficial to clean up the codebase. >>>> >>>> >>>> +1 (non-binding). >>>> >>>> >>>> Best, >>>> Wencong >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> At 2023-11-15 16:51:15, "Junrui Lee" <jrlee....@gmail.com> wrote: >>>> >Hi all, >>>> > >>>> >I'd like to start a discussion of FLIP-391: Deprecate >>>> >RuntimeContext#getExecutionConfig[1]. >>>> > >>>> >Currently, the FLINK RuntimeContext is important for connecting user >>>> >functions to the underlying runtime details. It provides users with >>>> >necessary runtime information during job execution. >>>> >However, he current implementation of the FLINK RuntimeContext exposes >>>> the >>>> >ExecutionConfig to users, resulting in two issues: >>>> >Firstly, the ExecutionConfig contains much unrelated information that >>>> can >>>> >confuse users and complicate management. >>>> >Secondly, exposing the ExecutionConfig allows users to modify it >>>> during job >>>> >execution, which can cause inconsistencies and problems, especially >>>> with >>>> >operator chaining. >>>> > >>>> >Therefore, we propose deprecating the >>>> RuntimeContext#getExecutionConfig in >>>> >the FLINK RuntimeContext. In the upcoming FLINK-2.0 version, we plan to >>>> >completely remove the RuntimeContext#getExecutionConfig method. >>>> Instead, we >>>> >will introduce alternative getter methods that enable users to access >>>> >specific information without exposing unnecessary runtime details. >>>> These >>>> >getter methods will include: >>>> > >>>> >1. @PublicEvolving <T> TypeSerializer<T> >>>> >createSerializer(TypeInformation<T> typeInformation); >>>> >2. @PublicEvolving Map<String, String> getGlobalJobParameters(); >>>> >3. @PublicEvolving boolean isObjectReuseEnabled(); >>>> > >>>> >Looking forward to your feedback and suggestions, thanks. >>>> > >>>> >[1] >>>> > >>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=278465937 >>>> > >>>> >Best regards, >>>> >Junrui >>>> >>>
