Hi Zhu, Thanks for you time for reviewing this.
Extending ´ExecutionConfig´ will allow to modify the values in the config (this is what we want to prevent with Option2) To extend the ExecutionConfig is not simpler to do Option1 (expose ExecutionConfig directly). Regards On 2023/04/03 09:42:28 Zhu Zhu wrote: > Hi João, > > Thanks for creating this FLIP! > I'm overall +1 for it to unblock the migration of sinks to SinkV2. > > Yet I think it's better to let the `ReadableExecutionConfig` extend > `ExecutionConfig`, because otherwise we have to introduce a new method > `TypeInformation#createSerializer(ReadableExecutionConfig)`. The new > method may require every `TypeInformation` to implement it, including > Flink built-in ones and custom ones, otherwise exceptions will happen. > That goal, however, is pretty hard to achieve. > > Thanks, > Zhu > > João Boto <eskabe...@apache.org> 于2023年2月28日周二 23:34写道： > > > > I have update the FLIP with the 2 options that we have discussed.. > > > > Option 1: Expose ExecutionConfig directly on InitContext > > this have a minimal impact as we only have to expose the new methods > > > > Option 2: Expose ReadableExecutionConfig on InitContext > > with this option we have more impact as we need to add a new method to > > TypeInformation and change all implementations (current exists 72 > > implementations) > > > > Waiting for feedback or concerns about the two options >
