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!"
>
>
>

Reply via email to