Hi Joao, Thanks for driving this FLIP, +1 for exposing a read-only ExecutionConfig, users/developers are not expected to modify the ExecutionConfig in Sink.
Can we directly introduce the read-only ExecutionConfig in this FLIP? No need for a separate FLIP, because currently it will only be used in InitContext and will not affect other user interfaces. WDYT? cc @Gunnar Best, Lijie João Boto <eskabe...@apache.org> 于2023年1月16日周一 19:22写道： > Hi Jing Ge, > Thanks for your response.. > > Making the review left above about all connectors, I realise that we need > the full ExecutionConfig as it is needed to generate the serializer > correctly if objectReuse is enabled as we call > TypeInformation.createSerializer(ExecutionConfig config) > > On the PoC we use the ExecutionConfig and I don't see this... :( > > Regards > > On 2023/01/14 00:01:52 Jing Ge wrote: > > Hi Joao, > > > > Thanks for bringing this up. Exposing internal domain instances depends > on > > your requirements. Technically, it is even possible to expose the > > RuntimeContext [1] (must be considered very carefully). Since you > mentioned > > that you only need to know if objectReuse is enabled, how about just > expose > > isObjectReuseEnabled instead of the whole ExecutionConfig? The idea is to > > shrink the scope as small as possible to satisfy the requirement. If more > > information from ExecutionConfig is needed later, we still can refactor > the > > code properly according to the strong motivation. > > > > Best regards, > > Jing > > > > [1] > > > https://github.com/apache/flink/blob/560b4612735a2b9cd3b5db88adf5cb223e85535b/flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/operators/sink/SinkWriterOperator.java#L279 > > > > On Fri, Jan 13, 2023 at 6:19 PM João Boto <eskabe...@apache.org> wrote: > > > > > Hi Gunnar, > > > Thanks for your time and response... > > > > > > I think the problem you want to solve is the exposure of the > > > ExecutionConfig (that can be mutated) no? > > > The configuration is not mutated, we only need to know if objectReuse > is > > > enable. > > > This is already expose on RuntimeContext we think to keep it similar > to it > > > to simplify any migrations, but as said, for this migration from > > > ExecutionConfig we only need the isObjectReuseEnabled, and we could > expose > > > only this configuration.. > > > > > > Best regards, > > > > > > > > > On 2023/01/13 15:50:09 Gunnar Morling wrote: > > > > Hey Joao, > > > > > > > > Thanks for this FLIP! One question on the proposed interface changes: > > > > is it expected that the configuration is *mutated* via the > InitContext > > > > passed to Sink::createWriter()? If that's not the case, how about > > > > establishing a read-only contract representing the current > > > > configuration and passing in that one instead? That would probably > > > > deserve its own FLIP upon which yours here then would depend. Later > > > > on, other contracts which effectively shouldn't modify a config could > > > > use that one, too. > > > > > > > > Note I don't mean to stall your efforts here, but I thought it'd be a > > > > good idea to bring it up and gauge the general interest in this. > > > > > > > > Best, > > > > > > > > --Gunnar > > > > > > > > Am Fr., 13. Jan. 2023 um 15:17 Uhr schrieb Joao Boto <b...@boto.pro > >: > > > > > > > > > > Hi flink devs, > > > > > > > > > > I'd like to start a discussion thread for FLIP-287[1]. > > > > > This comes from an offline discussion with @Lijie Wang, from > > > FLIP-239[2] > > > > > specially for the sink[3]. > > > > > > > > > > Basically to expose the ExecutionConfig and JobId on > > > SinkV2#InitContext. > > > > > This changes are necessary to correct migrate the current sinks to > > > SinkV2 > > > > > like JdbcSink, KafkaTableSink and so on, that relies on > RuntimeContext > > > > > > > > > > Comments are welcome! > > > > > Thanks, > > > > > > > > > > [1] > > > > > > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=240880853 > > > > > [2] > > > > > > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=217386271 > > > > > [3] https://issues.apache.org/jira/browse/FLINK-25421 > > > > > > > > > >
