Yair Zaslavsky has posted comments on this change. Change subject: 5. [WIP] core: Introducing configuration loader ......................................................................
Patch Set 4: (3 comments) http://gerrit.ovirt.org/#/c/24365/4/backend/manager/modules/extension-manager/src/main/java/org/ovirt/engine/core/extensions/mgr/ConfigurationLoader.java File backend/manager/modules/extension-manager/src/main/java/org/ovirt/engine/core/extensions/mgr/ConfigurationLoader.java: Line 79: return instance; Line 80: } Line 81: Line 82: Line 83: public void load(File directory) throws ConfigurationException { > I am unsure we need to accept File as parameter, but load these two locatio We can accept two directory parameters, one for the "built in" extensions (the /usr one) and one for the "custom" one (the /etc one) . the loading part should be quite the same for both directories. after we perform the loading part for each one of them, we can activate the extensions (see also my comment about "activate"). How does it sound? Line 84: // Check that the folder that contains the configuration files exists: Line 85: if (!directory.exists()) { Line 86: throw new ConfigurationException(new FileNotFoundException( Line 87: "The directory \"" + directory.getAbsolutePath() + "\" containing the configuration files doesn't " Line 105: Line 106: /** Line 107: * Activates the enabled configurations Line 108: */ Line 109: public void activate() { > not sure why we need activate... after you load all resources you can "acti So you want this code to be moved to the load method above, after all configurations are loaded? Line 110: for (ExtensionEntry entry: loadedEntries.values()) { Line 111: if (entry.enabled) { Line 112: activatedEntries.put(entry.name, entry); Line 113: List<ExtensionEntry> entries = serviceEntries.get(entry.service); Line 137: + "\" already has the name " Line 138: + name); Line 139: } Line 140: ExtensionEntry entry = Line 141: new ExtensionEntry(file, config.getBoolean(ENABLED, true), name, config.getString(SERVICE), config); > why the extension entry cannot get file and config, and extract what it nee Not sure I understand your comment. Why do you think it's a bad idea to perform during the file loading? where should I check if the configuration with this name is alredy loaded? Line 142: loadedEntries.put(entry.name, entry); Line 143: Line 144: } catch (IOException exception) { Line 145: throw new ConfigurationException( -- To view, visit http://gerrit.ovirt.org/24365 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I182904177ec088e62b35bde870ec79725fabc4e4 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> 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
