Thanks for pointing them out, Dawid. I've went over the overall doc again and corrected the above typos.
- ModuleManager#listFunctions() returns Set<String> - ModuleManager holds a LinkedHashMap<String, Module> to keep loaded modules in order - ModuleFactory#createModule(Map<String, String>) and returns Module On Thu, Oct 17, 2019 at 2:27 AM Dawid Wysakowicz <dwysakow...@apache.org> wrote: > Hi all, > > Generally I'm fine with the design. Before I cast my +1 I wanted to > clarify one thing. Is the module name in ModuleFactory#createModule > necessary? Can't it be just?: > > interface ModuleFactory extends TableFactory { > Module createModule(Map<String, String> properties); > } > > The name under which the module was registered should not affect the > implementation of the module as far as I can tell. Could we remove this > parameter from the method? > > I also spotted a few "bugs" in the design, but they do not affect the > outcome of the design, as they are either just artifacts of refactoring the > FLIP or affect only the internal implementation: > > - there is a typo in the ModuleFactory#createModule return type. It > should be Module instead of Plugin > - the return type of ModuleManager:listFunctions() should be > Set<String> instead of Set<Set<String>>, right? > - we cannot use list to store the modules in ModuleManager if I am not > mistaken. We need to store them in a Map to e.g. be able to unload the > modules by its name. > > Best, > > Dawid > On 17/10/2019 04:16, Jark Wu wrote: > > +1 > > Thanks, > Jark > > On Thu, 17 Oct 2019 at 04:44, Peter Huang <huangzhenqiu0...@gmail.com> > <huangzhenqiu0...@gmail.com> > wrote: > > > +1 Thanks > > On Wed, Oct 16, 2019 at 12:48 PM Xuefu Z <usxu...@gmail.com> > <usxu...@gmail.com> wrote: > > > +1 (non-biding) > > On Wed, Oct 16, 2019 at 2:26 AM Timo Walther <twal...@apache.org> > <twal...@apache.org> wrote: > > > +1 > > Thanks, > Timo > > > On 15.10.19 20:50, Bowen Li wrote: > > Hi all, > > I'd like to kick off a voting thread for FLIP-68: Extend Core Table > > System > > with Pluggable Modules [1], as we have reached consensus in [2]. > > The voting period will be open for at least 72 hours, ending at 7pm > > Oct > > 18 > > UTC. > > Thanks, > Bowen > > [1] > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-68%3A+Extend+Core+Table+System+with+Pluggable+Modules > > [2] https://www.mail-archive.com/dev@flink.apache.org/msg29894.html > > -- > Xuefu Zhang > > "In Honey We Trust!" > > >