xuyangzhong commented on code in PR #25963:
URL: https://github.com/apache/flink/pull/25963#discussion_r1914546250
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/ModuleFactory.java:
##########
@@ -80,48 +66,14 @@ interface Context {
}
default String factoryIdentifier() {
- if (requiredContext() == null || supportedProperties() == null) {
- throw new ModuleException("Module factories must implement
factoryIdentifier()");
- }
-
- return null;
+ throw new ModuleException("Module factories must implement
factoryIdentifier()");
}
default Set<ConfigOption<?>> requiredOptions() {
- if (requiredContext() == null || supportedProperties() == null) {
- throw new ModuleException("Module factories must implement
requiredOptions()");
- }
-
- return null;
+ throw new ModuleException("Module factories must implement
requiredOptions()");
}
default Set<ConfigOption<?>> optionalOptions() {
- if (requiredContext() == null || supportedProperties() == null) {
- throw new ModuleException("Module factories must implement
optionalOptions()");
- }
-
- return null;
- }
-
- //
--------------------------------------------------------------------------------------------
- // Default implementations for legacy {@link TableFactory} stack.
- //
--------------------------------------------------------------------------------------------
-
- /**
- * @deprecated Implement the {@link Factory} based stack instead.
- */
- @Deprecated
- default Map<String, String> requiredContext() {
- // Default implementation for modules implementing the new {@link
Factory} stack instead.
- return null;
- }
-
- /**
- * @deprecated Implement the {@link Factory} based stack instead.
- */
- @Deprecated
- default List<String> supportedProperties() {
- // Default implementation for modules implementing the new {@link
Factory} stack instead.
- return null;
+ throw new ModuleException("Module factories must implement
optionalOptions()");
Review Comment:
ditto
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/ModuleFactory.java:
##########
@@ -80,48 +66,14 @@ interface Context {
}
default String factoryIdentifier() {
- if (requiredContext() == null || supportedProperties() == null) {
- throw new ModuleException("Module factories must implement
factoryIdentifier()");
- }
-
- return null;
+ throw new ModuleException("Module factories must implement
factoryIdentifier()");
Review Comment:
Remove this default implement. See more at
https://lists.apache.org/thread/tlk64byvxtbkox3wyv2jfoqfdtf523bf
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/ModuleFactory.java:
##########
@@ -80,48 +66,14 @@ interface Context {
}
default String factoryIdentifier() {
- if (requiredContext() == null || supportedProperties() == null) {
- throw new ModuleException("Module factories must implement
factoryIdentifier()");
- }
-
- return null;
+ throw new ModuleException("Module factories must implement
factoryIdentifier()");
}
default Set<ConfigOption<?>> requiredOptions() {
- if (requiredContext() == null || supportedProperties() == null) {
- throw new ModuleException("Module factories must implement
requiredOptions()");
- }
-
- return null;
+ throw new ModuleException("Module factories must implement
requiredOptions()");
Review Comment:
ditto
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
