Hi Jane, Thanks for kicking off this work and collecting the detailed list.
+1 to add the missing annotation. This often confuses me whether the class can be modified without breaking the compatibility when looking at classes in table-common and table-api. Explicitly mark the visibility can be helpful in this case. I have some additional suggestions wrt the class annotations: - classes in org.apache.flink.table.catalog.stats.* can be @PublicEvolving, because all the classes in there are needed to build the @PublicEvolving CatalogColumnStatistics. - PeriodicCacheReloadTrigger and TimedCacheReloadTrigger can be @PublicEvolving, they are built-in implementations of cache reload trigger and are exposed to connectors. - CoreModule can be @PublicEvolving to allow users use it in TableEnv#loadModule(name, Module). Best, Jark On Fri, 21 Jul 2023 at 00:34, Jane Chan <qingyue....@gmail.com> wrote: > Hi, Devs, > > I would like to start a discussion on adding missing visibility annotation > (PublicEvolving / Internal etc.) for Table APIs. > > The motivation for starting this discussion was during the cleanup of which > Table API to be deprecated for version 2.0, I noticed that some of the APIs > lack visibility annotations, which may lead to users relying on APIs that > should have been marked as internal. > > Therefore, I have compiled a sheet[1] listing the currently unmarked > classes under the table-api-java, table-api-java-bridge, and table-common > modules and the recommended annotations to be added. > > My thought is that all public classes/interfaces within the three modules > mentioned above should be explicitly marked, and we might consider > introducing a new architectural rule to perform auto-check on newly added > APIs in the future. > > Let me explain the details. > > 1. Why table-api-java, table-api-java-bridge, and table-common? > Because according to Flink's application project configuration doc[2], > table-api-java and table-api-java-bridge are the leading dependencies for > users to develop a table program. Although flink-table-common is not > listed, it is the core dependency for users to implement a User-Defined > Function/Connector[3]. > > 2. How are the classes listed in this table compiled? > I use a customized Intellij profile to perform the code inspection under > these three modules to find all public classes/interfaces without API > visibility annotations, along with a manual check. > > 3. How is the suggested API visibility to be determined? > For all APIs suggested as PublicEvolving, I added a comment on the cell to > explain the reason. The rest APIs, which are indicated as Internal, are > either util classes or implementations. > > I'm looking forward to your ideas, and it would be great if any interested > developers could help review this list together. > > > [1] > > https://docs.google.com/spreadsheets/d/1e8M0tUtKkZXEd8rCZtZ0C6Ty9QkNaPySsrCgz0vEID4/edit?usp=sharing > [2] > > https://nightlies.apache.org/flink/flink-docs-master/docs/dev/configuration/overview/#which-dependencies-do-you-need > [3] > > https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sourcessinks/#project-configuration > > > Best regards, > Jane >
