[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #4202: server: don't export APIs if feature is not enabled in any zones

[GitBox <gi...@apache.org>]

GabrielBrascher commented on a change in pull request #4202:
URL: https://github.com/apache/cloudstack/pull/4202#discussion_r449932428



##########
File path: api/src/main/java/org/apache/cloudstack/backup/BackupManager.java
##########
@@ -38,7 +38,7 @@
     ConfigKey<Boolean> BackupFrameworkEnabled = new ConfigKey<>("Advanced", Boolean.class,
             "backup.framework.enabled",
             "false",
-            "Is backup and recovery framework enabled.", true, ConfigKey.Scope.Zone);
+            "Is backup and recovery framework enabled. Restart management server on global value change.", true, ConfigKey.Scope.Zone);

Review comment:
       According to the description, ADMIN needs to restart the management server and therefore this is not a dynamic setting. 
   
   What do you think of changing the `isDynamic` param to `false`?
   
   _ConfigKey constructor:_
   ```
   public ConfigKey(String category, Class<T> type, String name, String defaultValue, String description, boolean isDynamic, Scope scope) {
       this(type, name, category, defaultValue, description, isDynamic, scope, null);
   }
   ```




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org