+1, make sense. Best, Ron
Wencong Liu <liuwencle...@163.com> 于2023年7月27日周四 09:47写道： > Hi Matthias, > > Thanks for your reply. Due to my busy work reasons, I would like to focus > only on > the `Path` class in FLIP-347 for now. As for the implementation of other > modules, > I will review them when I have available time later on. > > > Best regards, > Wencong Liu > > > > > > > > > > > > > > > At 2023-07-26 18:35:21, "Matthias Pohl" <matthias.p...@aiven.io.INVALID> > wrote: > >Correct. I don't have the intention to block this FLIP if it's too much > >effort to expand it. Sorry if that's the message that came across. > > > >On Wed, Jul 26, 2023 at 12:17 PM Xintong Song <tonysong...@gmail.com> > wrote: > > > >> I think it worth looking into all implementations of > IOReadeableWritable. > >> However, I would not consider that as a concern of this FLIP. > >> > >> An important convention of the open-source community is volunteer work. > If > >> Wencong only wants to work on the `Path` case, I think he should not be > >> asked to investigate all other cases. > >> > >> I believe it's not Matthias's intention to put more workload on Wencong. > >> It's just sometimes such requests are not easy to say no. > >> > >> Best, > >> > >> Xintong > >> > >> > >> > >> On Wed, Jul 26, 2023 at 4:14 PM Matthias Pohl > >> <matthias.p...@aiven.io.invalid> wrote: > >> > >> > Is the time constraint driven by the fact that you wanted to have that > >> > effort being included in 1.18? If so, it looks like that's not > possible > >> > based on the decision being made for 1.18 to only allow document > changes > >> > [1]. So, there would be actually time to look into it. WDYT? > >> > > >> > [1] https://lists.apache.org/thread/7l1c9ybqgyc1mx7t7tk4wkc1cm8481o9 > >> > > >> > On Tue, Jul 25, 2023 at 12:04 PM Junrui Lee <jrlee....@gmail.com> > wrote: > >> > > >> > > +1 > >> > > > >> > > Best, > >> > > Junrui > >> > > > >> > > Jing Ge <j...@ververica.com.invalid> 于2023年7月24日周一 23:28写道： > >> > > > >> > > > agree, since we want to try our best to deprecate APIs in 1.18, it > >> > makes > >> > > > sense. > >> > > > > >> > > > > >> > > > Best regards, > >> > > > Jing > >> > > > > >> > > > On Mon, Jul 24, 2023 at 12:11 PM Wencong Liu < > liuwencle...@163.com> > >> > > wrote: > >> > > > > >> > > > > Hi Jing and Matthias, > >> > > > > > >> > > > > > >> > > > > I believe it is reasonable to examine all classes that > >> implement > >> > > the > >> > > > > IOReadableWritable > >> > > > > interface and summarize their actual usage. However, due to time > >> > > > > constraints, I suggest > >> > > > > we minimize the scope of this FLIP to focus on the Path class. > As > >> for > >> > > > > other components > >> > > > > that implement IOReadableWritable, we can make an effort to > >> > investigate > >> > > > > them > >> > > > > in the future. WDYT? > >> > > > > > >> > > > > > >> > > > > Best regards, > >> > > > > Wencong Liu > >> > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > At 2023-07-22 00:46:45, "Jing Ge" <j...@ververica.com.INVALID> > >> > wrote: > >> > > > > >Hi Wencong, > >> > > > > > > >> > > > > >Thanks for the clarification. I got your point. It makes sense. > >> > > > > > > >> > > > > >Wrt IOReadableWritable, the suggestion was to check all classes > >> that > >> > > > > >implemented it, e.g. BlockInfo, Value, Configuration, etc. Not > >> > limited > >> > > > to > >> > > > > >the Path. > >> > > > > > > >> > > > > >Best regards, > >> > > > > >Jing > >> > > > > > > >> > > > > >On Fri, Jul 21, 2023 at 4:31 PM Wencong Liu < > liuwencle...@163.com > >> > > >> > > > wrote: > >> > > > > > > >> > > > > >> Hello Jing, > >> > > > > >> > >> > > > > >> > >> > > > > >> Thanks for your reply. The URI field should be final and the > >> > > > > >> Path will be immutable.The static method > >> > > deserializeFromDataInputView > >> > > > > >> will create a new Path object instead of replacing the URI > field > >> > > > > >> in a existed Path Object. > >> > > > > >> > >> > > > > >> > >> > > > > >> For the crossing multiple modules issue, I've explained it in > >> the > >> > > > reply > >> > > > > >> to Matthias. > >> > > > > >> > >> > > > > >> > >> > > > > >> Best regards, > >> > > > > >> > >> > > > > >> > >> > > > > >> Wencong Liu > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> At 2023-07-21 18:05:26, "Jing Ge" <j...@ververica.com.INVALID > > > >> > > wrote: > >> > > > > >> >Hi Wencong, > >> > > > > >> > > >> > > > > >> >Just out of curiosity, will the newly introduced > >> > > > > >> >deserializeFromDataInputView() method make the Path mutable > >> > again? > >> > > > > >> > > >> > > > > >> >What Matthias suggested makes sense, although the extension > >> might > >> > > > make > >> > > > > >> this > >> > > > > >> >FLIP cross multiple modules. > >> > > > > >> > > >> > > > > >> >Best regards, > >> > > > > >> >Jing > >> > > > > >> > > >> > > > > >> >On Fri, Jul 21, 2023 at 10:23 AM Matthias Pohl > >> > > > > >> ><matthias.p...@aiven.io.invalid> wrote: > >> > > > > >> > > >> > > > > >> >> There's a kind-of-related issue FLINK-4758 [1] that > proposes > >> > > > removing > >> > > > > >> the > >> > > > > >> >> IOReadableWritable interface from more classes. It was > >> briefly > >> > > > > >> mentioned in > >> > > > > >> >> the must-have work items discussion [2]. > >> > > > > >> >> > >> > > > > >> >> I'm not too sure about the usage of IOReadableWritable: > >> > > ...whether > >> > > > it > >> > > > > >> would > >> > > > > >> >> go away with the removal of the DataSet API in general > (the > >> > Jira > >> > > > > issue > >> > > > > >> has > >> > > > > >> >> DataSet as a component), anyway. > >> > > > > >> >> > >> > > > > >> >> Otherwise, might it make sense to extend the scope of this > >> > FLIP? > >> > > > > >> >> > >> > > > > >> >> [1] https://issues.apache.org/jira/browse/FLINK-4758 > >> > > > > >> >> [2] > >> > > > https://lists.apache.org/thread/gf0h4gh3xfsj78cpdsxsnj70nhzcmv9r > >> > > > > >> >> > >> > > > > >> >> On Fri, Jul 21, 2023 at 6:04 AM Xintong Song < > >> > > > tonysong...@gmail.com> > >> > > > > >> >> wrote: > >> > > > > >> >> > >> > > > > >> >> > +1 > >> > > > > >> >> > > >> > > > > >> >> > Best, > >> > > > > >> >> > > >> > > > > >> >> > Xintong > >> > > > > >> >> > > >> > > > > >> >> > > >> > > > > >> >> > > >> > > > > >> >> > On Fri, Jul 21, 2023 at 10:54 AM Wencong Liu < > >> > > > liuwencle...@163.com > >> > > > > > > >> > > > > >> >> wrote: > >> > > > > >> >> > > >> > > > > >> >> > > Hi devs, > >> > > > > >> >> > > > >> > > > > >> >> > > I would like to start a discussion on FLIP-347: Remove > >> > > > > >> >> IOReadableWritable > >> > > > > >> >> > > serialization in Path [1]. > >> > > > > >> >> > > > >> > > > > >> >> > > > >> > > > > >> >> > > The Path class is currently mutable to support > >> > > > IOReadableWritable > >> > > > > >> >> > > serialization. However, many parts > >> > > > > >> >> > > of the code assume that the Path is immutable. By > making > >> > the > >> > > > Path > >> > > > > >> class > >> > > > > >> >> > > immutable, we can ensure > >> > > > > >> >> > > that paths are stored correctly without the > possibility > >> of > >> > > > > mutation > >> > > > > >> and > >> > > > > >> >> > > eliminate the occurrence of subtle errors. > >> > > > > >> >> > > As such I propose to modify the Path class to no > longer > >> > > > implement > >> > > > > >> the > >> > > > > >> >> > > IOReadableWritable interface. > >> > > > > >> >> > > Looking forward to your feedback. > >> > > > > >> >> > > [1] > >> > > > > >> >> > > > >> > > > > >> >> > > >> > > > > >> >> > >> > > > > >> > >> > > > > > >> > > > > >> > > > >> > > >> > https://cwiki.apache.org/confluence/display/FLINK/FLIP-347%3A+Remove+IOReadableWritable+serialization+in+Path > >> > > > > >> >> > > Best regards, > >> > > > > >> >> > > > >> > > > > >> >> > > > >> > > > > >> >> > > Wencong Liu > >> > > > > >> >> > > >> > > > > >> >> > >> > > > > >> > >> > > > > > >> > > > > >> > > > >> > > >> >
