Alon Bar-Lev has posted comments on this change. Change subject: aaa: Chages to ExtensionsManager ......................................................................
Patch Set 3: (4 comments) I think the engine extension manager is missing http://gerrit.ovirt.org/#/c/27785/3/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 120: private Map<String, BindingsLoader> bindingsLoaders = new HashMap<>(); Line 121: private Map<String, ExtensionEntry> loadedEntries = new HashMap<>(); Line 122: private ExtMap globalContext = new ExtMap().mput(Base.GlobalContextKeys.EXTENSIONS, new ArrayList<ExtMap>()); Line 123: Line 124: private Properties configProperties = new Properties(); can you please explain why we need this? Line 125: Line 126: public ExtMap getGlobalContext() { Line 127: return globalContext; Line 128: } Line 145: } Line 146: return result; Line 147: } Line 148: Line 149: public ExtensionsManager(Map<String, String> properties, List<File> extensionsDirectories, String applicationName) { why not just have empty? and call load externally for each property file? so we have: load(String file) -> load(File file) -> load(Properties props) moving the iteration of the load to EngineExtensionsManager. Line 150: bindingsLoaders.put(Base.ConfigBindingsMethods.JBOSSMODULE, new JBossBindingsLoader()); Line 151: this.configProperties.putAll(properties); Line 152: globalContext.put(Base.GlobalContextKeys.APPLICATION_NAME, applicationName); Line 153: http://gerrit.ovirt.org/#/c/27785/3/backend/manager/modules/pom.xml File backend/manager/modules/pom.xml: Line 17: <modules> Line 18: <module>extensions-api-root</module> Line 19: <module>uutils</module> Line 20: <module>compat</module> Line 21: <module>extensions-manager</module> you can move this even above the compat Line 22: <module>utils</module> Line 23: <module>common</module> Line 24: <module>dal</module> Line 25: <module>vdsbroker</module> http://gerrit.ovirt.org/#/c/27785/3/backend/manager/modules/utils/pom.xml File backend/manager/modules/utils/pom.xml: Line 73: <dependency> Line 74: <groupId>${engine.groupId}</groupId> Line 75: <artifactId>extensions-manager</artifactId> Line 76: <version>${engine.version}</version> Line 77: </dependency> why one above and one bellow? Line 78: Line 79: <dependency> Line 80: <groupId>javax.transaction</groupId> Line 81: <artifactId>jta</artifactId> -- 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: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
