[GitHub] [beam] kennknowles commented on pull request #12202: [BEAM-10407,10408] Schema Capable IO Table Provider Wrappers

2020-07-20 Thread GitBox
kennknowles commented on pull request #12202: URL: https://github.com/apache/beam/pull/12202#issuecomment-661432146 Drive by comment on commit history: clearly some of them should be squashed into one commit. I am not sure they all should be. After code review is done, it will be helpful

[GitHub] [beam] kennknowles commented on pull request #12202: [BEAM-10407,10408] Schema Capable IO Table Provider Wrappers

2020-07-14 Thread GitBox
kennknowles commented on pull request #12202: URL: https://github.com/apache/beam/pull/12202#issuecomment-658358581 I think Luke said the same thing. You put `TableProvider` interface somewhere everyone can depend on. I believe in your proposal it might end up in up in java core. I don't

[GitHub] [beam] kennknowles commented on pull request #12202: [BEAM-10407,10408] Schema Capable IO Table Provider Wrappers

2020-07-14 Thread GitBox
kennknowles commented on pull request #12202: URL: https://github.com/apache/beam/pull/12202#issuecomment-658357325 Didn't have time to look too closely, but once `TableProvider` moved to a service loader model, SQL should not have a `provided` dependency on the specific `TableProvider`