[GitHub] [flink] dianfu commented on issue #8956: [FLINK-12991][python] Correct the implementation of Catalog.get_table_factory
dianfu commented on issue #8956: [FLINK-12991][python] Correct the implementation of Catalog.get_table_factory URL: https://github.com/apache/flink/pull/8956#issuecomment-509447924 @bowenli86 Thanks a lot for the clarify. Make much sense to me. :) 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] dianfu commented on issue #8956: [FLINK-12991][python] Correct the implementation of Catalog.get_table_factory
dianfu commented on issue #8956: [FLINK-12991][python] Correct the implementation of Catalog.get_table_factory URL: https://github.com/apache/flink/pull/8956#issuecomment-509084620 @bowenli86 Thanks a lot for your reply. > I'm fine either way because open, close, getTableFactory are actually publicly available to Java/Scala Table API users. Regarding to `open, close, getTableFactory`, I agree with you that they are public in Java. However, I think they are public for Catalog developers, not for API users. I think it is also the same case for `TableFactory`. It's also public for connector/format/catalog developers, not for API users. I have not found any documentation about how to use `TableFactory` for API users. So the key point here is not only whether the interface is public or not, but also how we want users to use it. > Hiding them from Python table api only solve the problem from the surface, not from its root. According to the above, I don't agree with you on this point. We should remove this interface in Python for now as I think this interface is not for API users. Anyway, we can add it back in the future if we found that it's useful and should be supported. However, removing an interface isn't easy to do. @zentol Thanks a lot for the remind. The failure is not related to this PR and I have rebased the PR to re-trigger the tests. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] dianfu commented on issue #8956: [FLINK-12991][python] Correct the implementation of Catalog.get_table_factory
dianfu commented on issue #8956: [FLINK-12991][python] Correct the implementation of Catalog.get_table_factory URL: https://github.com/apache/flink/pull/8956#issuecomment-508610372 @bowenli86 Any feedback? We need to fix this issue in 1.9 and the 1.9 release branch will be cut latter today. So appreciate if you can take a look at this PR ASAP. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] dianfu commented on issue #8956: [FLINK-12991][python] Correct the implementation of Catalog.get_table_factory
dianfu commented on issue #8956: [FLINK-12991][python] Correct the implementation of Catalog.get_table_factory URL: https://github.com/apache/flink/pull/8956#issuecomment-508307779 @bowenli86 Thanks a lot for your comments. The Python catalog API is just a thin wrapper of the Java catalog API and it's not designed to let users implement a Python catalog. So for your questions, it's the latter case. Users should develop their own catalog in Java and reference them in Python. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] dianfu commented on issue #8956: [FLINK-12991][python] Correct the implementation of Catalog.get_table_factory
dianfu commented on issue #8956: [FLINK-12991][python] Correct the implementation of Catalog.get_table_factory URL: https://github.com/apache/flink/pull/8956#issuecomment-507906136 @bowenli86 Could you help to take a look at if this change makes sense to you? 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: us...@infra.apache.org With regards, Apache Git Services
