Alon Bar-Lev has posted comments on this change. Change subject: aaa: Changes to ExtensionsManager ......................................................................
Patch Set 18: (6 comments) I do not understand the mix between the commons-loggins and java.util.logging, can't you use the commons-loggings only. http://gerrit.ovirt.org/#/c/27785/18/backend/manager/modules/extensions-api-root/extensions-api/src/main/java/org/ovirt/engine/api/extensions/Base.java File backend/manager/modules/extensions-api-root/extensions-api/src/main/java/org/ovirt/engine/api/extensions/Base.java: Line 270: /** Line 271: * Invoke commands. Line 272: */ Line 273: public static class InvokeCommands { Line 274: - Line 275: /** Line 276: * Loads extension instance. Line 277: * Extension should configure its information within the context during this command. Line 278: */ Line 273: public static class InvokeCommands { Line 274: Line 275: /** Line 276: * Loads extension instance. Line 277: * Extension should configure its information within the context during this command. and validate its configuration Extension should not perform any operation that may fail or change system state at this point. Line 278: */ Line 279: public static final ExtUUID LOAD = new ExtUUID("EXTENSION_LOAD", "b0f2460e-7971-4a9c-b4e1-c1db1362a47a"); Line 280: /** Line 281: * Initialize extension instance. Line 278: */ Line 279: public static final ExtUUID LOAD = new ExtUUID("EXTENSION_LOAD", "b0f2460e-7971-4a9c-b4e1-c1db1362a47a"); Line 280: /** Line 281: * Initialize extension instance. Line 282: * Extension should initialize the extension based on the configuration missing dot Line 283: */ Line 284: public static final ExtUUID INITIALIZE = new ExtUUID("EXTENSION_INITIALIZE", "e5ae1b7f-9104-4f23-a444-7b9175ff68d2"); Line 285: /** Terminate extension instance. */ Line 286: public static final ExtUUID TERMINATE = new ExtUUID("EXTENSION_TERMINATE", "83152bf5-861f-46e8-b0d7-4d6da28303f8"); http://gerrit.ovirt.org/#/c/27785/18/backend/manager/modules/extensions-manager/src/main/java/org/ovirt/engine/core/extensions/mgr/ExtensionsManager.java File backend/manager/modules/extensions-manager/src/main/java/org/ovirt/engine/core/extensions/mgr/ExtensionsManager.java: Line 18: import java.util.logging.Level; Line 19: import java.util.logging.Logger; Line 20: import java.util.logging.LogManager; Line 21: Line 22: import org.apache.commons.logging.LogFactory; I do not understand why we need both commons logging and util.logging. Line 23: import org.jboss.modules.Module; Line 24: import org.jboss.modules.ModuleIdentifier; Line 25: import org.jboss.modules.ModuleLoadException; Line 26: import org.jboss.modules.ModuleLoader; Line 117: return file != null ? file.getAbsolutePath() : "N/A"; Line 118: } Line 119: } Line 120: Line 121: private static final Logger log = LogManager.getLogManager().getLogger(ExtensionsManager.class.getSimpleName()); why simple name? why not ExtensionManager.class sa we always do? private static final Log log = LogFactory.getLog(XXXX.class); Line 122: private static final Logger traceLog = LogManager.getLogManager().getLogger(getTraceLog()); Line 123: private Map<String, BindingsLoader> bindingsLoaders = new HashMap<>(); Line 124: private ConcurrentMap<String, ExtensionEntry> loadedEntries = new ConcurrentHashMap<>(); Line 125: private ExtMap globalContext = new ExtMap().mput(Base.GlobalContextKeys.EXTENSIONS, new ArrayList<ExtMap>()); Line 156: bindingsLoaders.put(Base.ConfigBindingsMethods.JBOSSMODULE, new JBossBindingsLoader()); Line 157: } Line 158: Line 159: public synchronized String load(Properties configuration) { Line 160: return loadImpl(configuration, null); won't it better to sync the loadImpl? Line 161: } Line 162: Line 163: public String load(File file) { Line 164: try (FileInputStream inputStream = new FileInputStream(file)) { -- To view, visit http://gerrit.ovirt.org/27785 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1c914df29a0dbf52ff6d2f8149687b31b4faffe1 Gerrit-PatchSet: 18 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
