PakhomovAlexander commented on code in PR #2386:
URL: https://github.com/apache/ignite-3/pull/2386#discussion_r1280741754
##########
modules/configuration-api/src/main/java/org/apache/ignite/configuration/RootKey.java:
##########
@@ -38,8 +38,10 @@ public class RootKey<T extends ConfigurationTree<VIEWT, ?>,
VIEWT> {
/** Schema class for the root. */
private final Class<?> schemaClass;
- /** Marked with {@link InternalConfiguration}. */
- private final boolean internal;
+ /** Marked with {@link ConfigurationExtension}. */
+ private final boolean privateExtension;
+
+ private final boolean publicExtension;
Review Comment:
Can you add javadoc here as well?
##########
modules/configuration-api/src/main/java/org/apache/ignite/configuration/annotation/ConfigurationExtension.java:
##########
@@ -39,5 +40,12 @@
@Target(TYPE)
@Retention(RUNTIME)
@Documented
-public @interface InternalConfiguration {
+public @interface ConfigurationExtension {
+
+ /**
+ * Controls whether this configuration is part of the public configuration
or is hidden from the end user.
+ * The extension is internal by default.
+ */
+ boolean internal() default true;
Review Comment:
Do you think `external` word is a better naming? By default, `external =
false` which is common default boolean value.
##########
modules/configuration-api/src/main/java/org/apache/ignite/configuration/RootKey.java:
##########
@@ -38,8 +38,10 @@ public class RootKey<T extends ConfigurationTree<VIEWT, ?>,
VIEWT> {
/** Schema class for the root. */
private final Class<?> schemaClass;
- /** Marked with {@link InternalConfiguration}. */
- private final boolean internal;
+ /** Marked with {@link ConfigurationExtension}. */
Review Comment:
Could you please mention that it is not only marked with
`ConfigurationExtension` but also with flag internal=true.
##########
modules/configuration-api/src/main/java/org/apache/ignite/configuration/RootKey.java:
##########
@@ -38,8 +38,10 @@ public class RootKey<T extends ConfigurationTree<VIEWT, ?>,
VIEWT> {
/** Schema class for the root. */
private final Class<?> schemaClass;
- /** Marked with {@link InternalConfiguration}. */
- private final boolean internal;
+ /** Marked with {@link ConfigurationExtension}. */
+ private final boolean privateExtension;
+
+ private final boolean publicExtension;
Review Comment:
UPD: I think it is enough to have a single field indicating if it is a
public or private extension.
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationUtil.java:
##########
@@ -594,6 +582,18 @@ public static Map<Class<?>, Set<Class<?>>>
polymorphicSchemaExtensions(Collectio
return schemaExtensions(extensions, PolymorphicConfigInstance.class);
}
+ /**
+ * Get configuration schemas and their validated internal extensions.
Review Comment:
```suggestion
* Get configuration schemas and their validated extensions.
```
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/RootInnerNode.java:
##########
@@ -28,8 +28,11 @@ public class RootInnerNode {
/** Root node. */
private final InnerNode node;
- /** Internal configuration. */
- private final boolean internal;
+ /** Internal configuration extensions. */
+ private final boolean privateExtension;
+
+ /** Public configuration extensions. */
+ private final boolean publicExtension;
Review Comment:
I don't see any reason to keep two booleans instead of one.
--
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]