ibessonov commented on a change in pull request #273:
URL: https://github.com/apache/ignite-3/pull/273#discussion_r689293965



##########
File path: 
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
##########
@@ -255,75 +227,36 @@ public void initialize(ConfigurationType storageType) 
throws ConfigurationValida
                 throw (ConfigurationChangeException)cause;
 
             throw new ConfigurationChangeException(
-                "Failed to write defalut configuration values into the storage 
" + configurationStorage.getClass(), e
+                "Failed to write default configuration values into the storage 
" + storage.getClass(), e
             );
         }
         catch (InterruptedException e) {
             throw new ConfigurationChangeException(
-                "Failed to initialize configuration storage " + 
configurationStorage.getClass(), e
+                "Failed to initialize configuration storage " + 
storage.getClass(), e
             );
         }
     }
 
     /**
      * Changes the configuration.
+     *
      * @param source Configuration source to create patch from.
-     * @param storage Expected storage for the changes. If null, a derived 
storage will be used
-     * unconditionaly.
      * @return Future that is completed on change completion.
      */
-    public CompletableFuture<Void> change(
-        ConfigurationSource source,
-        @Nullable ConfigurationStorage storage
-    ) {
-        Set<ConfigurationType> storagesTypes = new HashSet<>();
-
-        ConstructableTreeNode collector = new ConstructableTreeNode() {
-            @Override public void construct(String key, ConfigurationSource 
src) throws NoSuchElementException {
-                RootKey<?, ?> rootKey = rootKeys.get(key);
-
-                if (rootKey == null)
-                    throw new NoSuchElementException(key);
-
-                storagesTypes.add(rootKey.type());
-            }
-
-            @Override public ConstructableTreeNode copy() {
-                throw new UnsupportedOperationException("copy");
-            }
-        };
-
+    public CompletableFuture<Void> change(ConfigurationSource source) {
         source.reset();

Review comment:
       This call can be removed as well

##########
File path: 
modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/SchemaConfigurationConverterTest.java
##########
@@ -68,9 +71,10 @@
     @BeforeEach
     public void createRegistry() throws ExecutionException, 
InterruptedException {
         confRegistry = new ConfigurationRegistry(
-            Collections.singleton(TablesConfiguration.KEY),
-            Collections.singletonMap(TableValidator.class, 
Collections.singleton(SchemaTableValidatorImpl.INSTANCE)),
-            Collections.singleton(new 
TestConfigurationStorage(ConfigurationType.DISTRIBUTED)));
+            List.of(TablesConfiguration.KEY),
+            Map.of(TableValidator.class, 
Set.of(SchemaTableValidatorImpl.INSTANCE)),

Review comment:
       I meant "SchemaTableValidatorImpl.INSTANCE"

##########
File path: 
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationRegistry.java
##########
@@ -187,20 +176,27 @@ public void startStorageConfigurations(ConfigurationType 
storageType) {
 
     /**
      * Change configuration.
-     * @param changesSource Configuration source to create patch from it.
-     * @param storage Expected storage for the changes. Can be null, this will 
mean that derived storage will be used
-     * unconditionaly.
+     *
+     * @param changesSrc Configuration source to create patch from it.
      * @return Future that is completed on change completion.
      */
-    public CompletableFuture<Void> change(ConfigurationSource changesSource, 
@Nullable ConfigurationStorage storage) {
-        return changer.change(changesSource, storage);
+    public CompletableFuture<Void> change(ConfigurationSource changesSrc) {
+        return changer.change(changesSrc);
     }
 
-    /** */
-    private @NotNull CompletableFuture<Void> notificator(SuperRoot 
oldSuperRoot, SuperRoot newSuperRoot, long storageRevision) {
+    /**
+     * Configuration change notifier.
+     *
+     * @param oldSuperRoot Old roots values. All these roots always belong to 
a single storage.
+     * @param newSuperRoot New values for the same roots as in {@code oldRoot}.
+     * @param storageRevision Revision of the storage.
+     * @return Future that must signify when processing is completed. 
Exceptional completion is not expected.

Review comment:
       You know what, I completely forgot about this... We should fail this 
future if we encounter any exception in "futures" list inside of this method. 
Can you please fix it?




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


Reply via email to