[GitHub] [flink] Airblader commented on a change in pull request #17898: [FLINK-25047][table] Resolve architectural violations

[GitBox <gi...@apache.org>]

Airblader commented on a change in pull request #17898:
URL: https://github.com/apache/flink/pull/17898#discussion_r756268152



##########
File path: flink-table/flink-table-common/src/main/java/org/apache/flink/table/api/NoMatchingTableFactoryException.java
##########
@@ -28,6 +29,7 @@
 import java.util.stream.Collectors;
 
 /** Exception for not finding a {@link TableFactory} for the given properties. */
+@PublicEvolving
 public class NoMatchingTableFactoryException extends RuntimeException {

Review comment:
       > perhaps we should rather move it outside the api package as it's not referenced by any class in this package
   
   The idea is to have APIs live in the `api` package, though, so if it is API (see below), then it's the other stuff that should rather live elsewhere.
   
   > but only by internal classes TableFactoryService and FactoryUtil
   
   `FactoryUtil` is not internal, it is public API. And this is a meaningful exception returned from there, so I think it would be better to declare this part of the API and leave it where it is (since it is more correct and moving classes is breaking).




-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org