hequn8128 edited a comment on issue #8050: [FLINK-11067][table] Convert TableEnvironments to interfaces URL: https://github.com/apache/flink/pull/8050#issuecomment-479572670 @dawidwys Thanks a lot for your review. I think you have given a lot of good suggestions! 1. As for the useless APIs in `TableEnvironment` interface, maybe we can deprecate first and remove them later. This can make our interface compatible. 2. To uncouple the `Descriptor` with `TableEnvironment`, we can change registerTableSource to getTableSource in `RegistrableDescriptor`(also should rename `RegistrableDescriptor`). Once we get the table source or sink, then we can register the table source or sink with TableEnvironment. Although this makes users impossible to register table source(or sink) directly, I am ok with it. 3. For the `TableFactoryUtil`, I am not sure if we can port it into api or common module(I will make sure later). However, I do find we can also uncouple TableEnvironment here. In the current implementation, it seems table env is only used to decide whether it is a batch or stream source(or sink). We can also use descriptor to decide, i.e., a StreamTableDescriptor means a stream source(or sink). For 2 and 3, I think the biggest obstacle is we may need to keep backward compatible while converting TableEnvironment into an interface. @twalthr Would be great to also have your opinions here. Looking forward to having your suggestions. Thank you. Best, Hequn
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
