Hi Shammon, If we take a look at the JDK Event design as a reference, we can even add an Object into the event [1]. Back to the CatalogModificationEvent, everything related to the event could be defined in the Event. If we want to group some information into the Context, we could also consider adding the CatalogModificationContext into the Event and make the onEvent() method cleaner with only one input parameter CatalogModificationEvent, because the interface CatalogModificationListener is the most often used interface for users. Just my two cents.
Best regards, Jing [1] http://www.java2s.com/example/java-src/pkg/java/util/eventobject-85298.html On Thu, Jun 8, 2023 at 7:50 AM Shammon FY <zjur...@gmail.com> wrote: > 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/#/ > > > > >>> > > > > >>> > > > > >> > > > > > > > > > > > > > >
