Alon Bar-Lev has posted comments on this change.

Change subject: 5. [WIP] core: Introducing configuration loader
......................................................................


Patch Set 4:

(1 comment)

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 {
> EngineLocalConfig exists at utils module. We do not want extension-manager 
Why not depend on it? it is the only mean we currently use to access 
configuration.

I do not care if you pass these two directories into the constructor... but 
this means that the class cannot be singleton... right?

Which reminds me... what happens if someone calls load twice... :)

Anyway, you must have access to configuration for the disable/enable magic 
within configuration, and probably ordering in future.
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 "


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

Reply via email to