Hi, To @Jing Ge > Thanks for the clarification. Just out of curiosity, if the context is not part of the event, why should it be the input parameter of each onEvent call?
I think it's quite strange to put some information in an Event, such as a factory identifier for catalog, but they will be used by the listener. I place it in the context class and I think it is more suitable than directly placing it in the event class. To @Mason > 1. I'm also curious about default implementations. Would atlas/datahub be supported by default? We won't do that and external systems such as atlas/datahub need to implement the listener themselves. > 2. The FLIP title is confusing to me, especially in distinguishing it from FLIP-314. Would a better FLIP title be "Support Catalog Metadata Listener" or something alike? Thanks, I think "Support Catalog Modification Listener" will be more suitable, I'll update the title to it. Best, Shammon FY On Thu, Jun 8, 2023 at 12:25 PM Mason Chen <mas.chen6...@gmail.com> wrote: > Hi Shammon, > > FLIP generally looks good and I'm excited to see this feature. > > 1. I'm also curious about default implementations. Would atlas/datahub be > supported by default? > 2. The FLIP title is confusing to me, especially in distinguishing it from > FLIP-314. Would a better FLIP title be "Support Catalog Metadata Listener" > or something alike? > > Best, > Mason > > On Tue, Jun 6, 2023 at 3:33 AM Jing Ge <j...@ververica.com.invalid> wrote: > > > Hi Shammon, > > > > Thanks for the clarification. Just out of curiosity, if the context is > not > > part of the event, why should it be the input parameter of each onEvent > > call? > > > > Best regards, > > Jing > > > > On Tue, Jun 6, 2023 at 11:58 AM Leonard Xu <xbjt...@gmail.com> wrote: > > > > > Thanks Shammon for the timely update, the updated FLIP looks good to > me. > > > > > > Hope to see the vote thread and following FLIP-314 discussion thread. > > > > > > Best, > > > Leonard > > > > > > > On Jun 6, 2023, at 5:04 PM, Shammon FY <zjur...@gmail.com> wrote: > > > > > > > > Hi, > > > > > > > > Thanks for all the feedback. > > > > > > > > For @Jing Ge, > > > > I forget to update the demo code in the FLIP, the method is > > > > `onEvent(CatalogModificationEvent, CatalogModificationContext)` and > > there > > > > is no `onEvent(CatalogModificationEvent)`. I have updated the code. > > > Context > > > > contains some additional information that is not part of an Event, > but > > > > needs to be used in the listener, so we separate it from the event. > > > > > > > > For @Panagiotis, > > > > I think `ioExecutor` make sense to me and I have added it in > > > > `ContextModificationContext`, thanks > > > > > > > > For @Leonard, > > > > Thanks for your input. > > > > 1. I have updated `CatalogModificationContext` as an interface, as > well > > > as > > > > Context in CatalogModificationListenerFactory > > > > 2. Configuration sounds good to me, I have updated the method name > and > > > > getConfiguration in Context > > > > > > > > For @David, > > > > Yes, you're right. The listener will only be used on the client side > > and > > > > won't introduce a new code path for running per-job/per-session jobs. > > The > > > > listener will be created in `TableEnvironment` and `SqlGateway` which > > > can a > > > > `CatalogManager` with the listener. > > > > > > > > > > > > Best, > > > > Shammon FY > > > > > > > > > > > > On Tue, Jun 6, 2023 at 3:33 PM David Morávek < > david.mora...@gmail.com> > > > > wrote: > > > > > > > >> Hi, > > > >> > > > >> Thanks for the FLIP! Data lineage is an important problem to tackle. > > > >> > > > >> Can you please expand on how this is planned to be wired into the > > > >> JobManager? As I understand, the listeners will be configured > globally > > > (per > > > >> cluster), so this won't introduce a new code path for running > per-job > > / > > > >> per-session user code. Is that correct? > > > >> > > > >> Best, > > > >> D > > > >> > > > >> On Tue, Jun 6, 2023 at 9:17 AM Leonard Xu <xbjt...@gmail.com> > wrote: > > > >> > > > >>> Thanks Shammon for driving this FLIP forward, I’ve several comments > > > about > > > >>> the updated FLIP. > > > >>> > > > >>> 1. CatalogModificationContext is introduced as a class instead of > an > > > >>> interface, is it a typo? > > > >>> > > > >>> 2. The FLIP defined multiple Map<String, String> config(); > methods > > in > > > >>> some Context classes, Could we use Configuration > > > >> getConfiguration()；Class > > > >>> org.apache.flink.configuration.Configuration is recommend as it’s > > > public > > > >>> API and offers more useful methods as well. > > > >>> > > > >>> 3. The Context of CatalogModificationListenerFactory should be an > > > >>> interface too, and getUserClassLoder() > > > >>> would be more aligned with flink’s naming style. > > > >>> > > > >>> > > > >>> Best, > > > >>> Leonard > > > >>> > > > >>>> On May 26, 2023, at 4:08 PM, Shammon FY <zjur...@gmail.com> > wrote: > > > >>>> > > > >>>> Hi devs, > > > >>>> > > > >>>> We would like to bring up a discussion about FLIP-294: Support > > > >> Customized > > > >>>> Job Meta Data Listener[1]. We have had several discussions with > Jark > > > >> Wu, > > > >>>> Leonard Xu, Dong Lin, Qingsheng Ren and Poorvank about the > functions > > > >> and > > > >>>> interfaces, and thanks for their valuable advice. > > > >>>> The overall job and connector information is divided into metadata > > and > > > >>>> lineage, this FLIP focuses on metadata and lineage will be > discussed > > > in > > > >>>> another FLIP in the future. In this FLIP we want to add a > customized > > > >>>> listener in Flink to report catalog modifications to external > > metadata > > > >>>> systems such as datahub[2] or atlas[3]. Users can view the > specific > > > >>>> information of connectors such as source and sink for Flink jobs > in > > > >> these > > > >>>> systems, including fields, watermarks, partitions, etc. > > > >>>> > > > >>>> Looking forward to hearing from you, thanks. > > > >>>> > > > >>>> > > > >>>> [1] > > > >>>> > > > >>> > > > >> > > > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-294%3A+Support+Customized+Job+Meta+Data+Listener > > > >>>> [2] https://datahub.io/ > > > >>>> [3] https://atlas.apache.org/#/ > > > >>> > > > >>> > > > >> > > > > > > > > >
