Hi Timo,
Thank you for your feedback. I have updated the FLIP to address your
questions and concerns. Please let me know if you have any other feedback.
Thanks,
Yash Anand
On Tue, Mar 11, 2025 at 5:55 AM Timo Walther <twal...@apache.org> wrote:
> Hi Yash,
>
> thanks for updating the FLIP.
>
> Here is more feedback from my side:
>
> Add `option(ConfigOption<T> configOption, T value)` similar to
> `TableDescriptor`.
>
> Use `comment()` instead of `setComment` to be in sync with
> `TableDescriptor`.
>
> In `TableDescriptor`, the schema is not part of the `newBuilder` but
> offered as optional method `schema()`, this allows for omitting the
> schema and automatically derive it from the input. Do we want to offer
> the same functionality? We might want to offer a CREATE MODEL AS syntax
> in Table API?
>
> In `TableDescriptor.forConnector()` we make the connector option
> mandatory, is there no similar mandatory option for models?
>
> Similar to Leonard, I will not directly -1 to cancel existing voting
> process, but I hope to continue voting after the addressed above. For
> the next time please make sure that the DISCUSS thread has settled
> before voting.
>
> Thanks,
> Timo
>
>
> On 11.03.25 09:25, Leonard Xu wrote:
> > Thanks @Anand for the quick response and update, the updated FLIP looks
> clear enough to me.
> >
> > +1(binding)
> >
> > Best,
> > Leonard
> >
> >
> >> 2025年3月11日 12:46，Yash Anand <yan...@confluent.io.INVALID> 写道：
> >>
> >> Hi Leonard,
> >>
> >> Thank you for your input. I will update the FLIP accordingly to make it
> >> more clear and standardized enough.
> >>
> >> Thanks,
> >> Yash Anand
> >>
> >>
> >>
> >> On Mon, Mar 10, 2025 at 11:08 PM Leonard Xu <xbjt...@gmail.com> wrote:
> >>
> >>> Sorry for jumping the thread late, but I think current status of this
> FLIP
> >>> is not ready, at least for me
> >>>
> >>> (1) Could you finish your proposed API according Flink API bylaws? For
> >>> example the code piece should be:
> >>> Builder<SELF> {
> >>> SELF option(String key, String value
> >>> SELF setComment(@Nullable String comment);
> >>> =>
> >>> /** Builder for {@link ModelDescriptor}. **/
> >>> @PublicEvolving
> >>> Builder<SELF> {
> >>> /** Defines the option of {@link ModelDescriptor}. **/
> >>> SELF option(String key, String value);
> >>> /** Defines the comment of {@link ModelDescriptor}. **/
> >>> SELF setComment(@Nullable String comment);
> >>> (2) TableEnvironment is a public API, so any changes (such as adding
> >>> public methods in this case) must be clearly documented. You may refer
> to
> >>> [1] as an example. In the [Public Interfaces] section of this FLIP,
> only
> >>> TableEnvironment is listed. However, the subsequent [Proposed Changes]
> >>> section appears to conflate TableEnvironment with ModelDescriptor.
> >>> Clarifications are needed:
> >>> Which package should ModelDescriptor belong to?
> >>> Is ModelDescriptor intended to be an inner class of TableEnvironment?
> >>>
> >>> At last, this is a useful FLIP and I generally agree with the
> motivation
> >>> and design, but it is not clear and standardized enough.
> >>> I will not directly -1 to cancel existing voting process, but I hope to
> >>> continue voting after the addressed above(1)(2) comments. WDYT?
> >>>
> >>> Best,
> >>> Leonard
> >>> [1]
> >>>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=334760466
> >>>
> >>>
> >>>
> >>>
> >>>> 2025年3月11日 01:19，Yash Anand <yan...@confluent.io.INVALID> 写道：
> >>>>
> >>>> Hi Timo,
> >>>>
> >>>> Thanks for pointing that out. I have added the full API of
> >>>> the ModelDescriptor in the FLIP.
> >>>>
> >>>> Thanks,
> >>>> Yash Anand
> >>>>
> >>>>
> >>>> On Mon, Mar 10, 2025 at 11:27 AM Timo Walther <twal...@apache.org>
> >>> wrote:
> >>>>
> >>>>> Hi Yash,
> >>>>>
> >>>>> could you provide the full API of the ModelDescriptor in the FLIP?
> >>>>>
> >>>>> Thanks,
> >>>>> Timo
> >>>>>
> >>>>>
> >>>>> On 10.03.25 16:55, Mingge Deng wrote:
> >>>>>> Thanks Yash!
> >>>>>>
> >>>>>> +1 (binding)
> >>>>>>
> >>>>>> Best,
> >>>>>> Mingge
> >>>>>>
> >>>>>>
> >>>>>> On Mon, Mar 10, 2025 at 8:51 AM Dawid Wysakowicz <
> >>> dwysakow...@apache.org
> >>>>>>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> +1 (binding)
> >>>>>>> Best,
> >>>>>>> Dawid
> >>>>>>>
> >>>>>>> On Wed, 19 Feb 2025 at 18:37, Hao Li <h...@confluent.io.invalid>
> >>> wrote:
> >>>>>>>
> >>>>>>>> +1 (non-binding)
> >>>>>>>>
> >>>>>>>> Thanks Yash,
> >>>>>>>> Hao
> >>>>>>>>
> >>>>>>>> On Tue, Feb 18, 2025 at 10:46 AM Yash Anand
> >>>>> <yan...@confluent.io.invalid
> >>>>>>>>
> >>>>>>>> wrote:
> >>>>>>>>
> >>>>>>>>> Hi Everyone,
> >>>>>>>>>
> >>>>>>>>> I'd like to start a vote on FLIP-507: Add Model DDL methods in
> TABLE
> >>>>>>> API
> >>>>>>>>> [1] which has been discussed in this thread [2].
> >>>>>>>>>
> >>>>>>>>> The vote will be open for at least 72 hours unless there is an
> >>>>>>> objection
> >>>>>>>> or
> >>>>>>>>> not enough votes.
> >>>>>>>>>
> >>>>>>>>> [1]
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>
> >>>
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-507%3A+Add+Model+DDL+methods+in+TABLE+API
> >>>>>>>>> [2]
> >>> https://lists.apache.org/thread/w9dt6y1w0yns5j3g4685tstjdg5flvy9
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >
>
>
