ibessonov commented on code in PR #4244:
URL: https://github.com/apache/ignite-3/pull/4244#discussion_r1724676480
##########
modules/configuration-api/src/main/java/org/apache/ignite/configuration/SuperRootChange.java:
##########
@@ -17,20 +17,17 @@
package org.apache.ignite.configuration;
-import org.apache.ignite.configuration.ConfigurationTree;
-import org.apache.ignite.configuration.RootKey;
-
/**
* Interface that represent a "change" for the conjunction of all roots in the
configuration.
*/
public interface SuperRootChange {
/**
* Returns a root view for the root key.
*/
- <V> V viewRoot(RootKey<? extends ConfigurationTree<V, ?>, V> rootKey);
+ <V, C, T extends ConfigurationTree<? super V, ? super C>> V
viewRoot(RootKey<T, V> rootKey);
Review Comment:
What is this for?
##########
docs/_docs/developers-guide/clients/cpp.adoc:
##########
@@ -147,24 +147,27 @@ Here is how the client connector configuration looks like:
[source, json]
----
-"clientConnector": {
- "port": 10800,
- "idleTimeout":3000,
- "sendServerExceptionStackTraceToClient":true,
- "ssl": {
- enabled: true,
- clientAuth: "require",
- keyStore: {
- path: "KEYSTORE_PATH",
- password: "SSL_STORE_PASS"
- },
- trustStore: {
- path: "TRUSTSTORE_PATH",
- password: "SSL_STORE_PASS"
+{
Review Comment:
Since we're fixing it anyway, we could remove quotations from the example,
making it a proper HOCON rather than a JSON. Maybe it's worth doing as a
separate JIRA, to make merge conflicts simpler. What do you think?
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationRegistry.java:
##########
@@ -136,7 +136,7 @@ public void initializeConfigurationWith(ConfigurationSource
configurationSource)
* @param <T> Configuration tree type.
* @return Public configuration tree.
*/
- public <V, C, T extends ConfigurationTree<V, C>> T
getConfiguration(RootKey<T, V> rootKey) {
+ public <V, C, T extends ConfigurationTree<? super V, C>> T
getConfiguration(RootKey<T, V> rootKey) {
Review Comment:
I also doubt that this is required, as well as all the rest of `? super`,
there should be an easier way
##########
modules/cli/src/integrationTest/java/org/apache/ignite/internal/NodeConfig.java:
##########
@@ -44,27 +44,25 @@ public class NodeConfig {
* @return Config pattern.
*/
public static String restSslBootstrapConfig(@Nullable String ciphers) {
- return "{\n"
- + " network: {\n"
- + " port: {},\n"
- + " nodeFinder: {\n"
- + " netClusterNodes: [ {} ]\n"
- + " },\n"
- + " },\n"
- + " clientConnector.port: {} ,\n"
- + " rest: {\n"
+ return "ignite {\n"
+ + " network {\n"
+ + " port: {}\n"
+ + " nodeFinder.netClusterNodes: [ {} ]\n"
+ + " }\n"
+ + " clientConnector.port: {}\n"
+ + " rest {\n"
Review Comment:
Changing the rest of these lines wasn't really necessary, what motivated you
to do more changes than you really needed?
It's everywhere in this PR, I bet it can be 3 times smaller if you remove
all unnecessary changes
##########
modules/configuration-api/src/main/java/org/apache/ignite/configuration/RootKey.java:
##########
@@ -28,7 +28,7 @@
* @param <T> Type of the configuration tree described by the root key.
* @param <VIEWT> Type of the immutable snapshot view associated with the tree.
*/
-public class RootKey<T extends ConfigurationTree<VIEWT, ?>, VIEWT> {
+public class RootKey<T extends ConfigurationTree<? super VIEWT, ?>, VIEWT> {
Review Comment:
I think we don't need this change
--
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]
