Yair Zaslavsky has posted comments on this change.

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


Patch Set 16:

(4 comments)

http://gerrit.ovirt.org/#/c/24365/16/backend/manager/modules/extension-manager/src/main/java/org/ovirt/engine/core/extensions/mgr/Configuration.java
File 
backend/manager/modules/extension-manager/src/main/java/org/ovirt/engine/core/extensions/mgr/Configuration.java:

Line 66: 
Line 67:     /**
Line 68:      * Gets the properties object that is associated with the 
configuration hierarchy
Line 69:      *
Line 70:      *
> ?
Done
Line 71:      * @return the reference to the properties object
Line 72:      */
Line 73:     public abstract Properties getProperties();
Line 74: 


http://gerrit.ovirt.org/#/c/24365/16/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 23: /**
Line 24:  * This class is responsible for loading the required {@code 
Configuration} in order to create an extension. It holds
Line 25:  * the logic of ordering and solving conflicts during loading the 
configuration
Line 26:  */
Line 27: public class ConfigurationLoader {
> this is ExtensionLoader
Will fix that.
Line 28: 
Line 29:     private static final String NAME = "ovirt.engine.extension.name";
Line 30:     private static final String SERVICE = 
"ovirt.engine.extension.service";
Line 31:     private static final String ENABLED = 
"ovirt.engine.extension.enabled";


Line 29:     private static final String NAME = "ovirt.engine.extension.name";
Line 30:     private static final String SERVICE = 
"ovirt.engine.extension.service";
Line 31:     private static final String ENABLED = 
"ovirt.engine.extension.enabled";
Line 32:     private static final String MODULE = 
"ovirt.engine.extension.module";
Line 33:     private static final String TYPE = "ovirt.engine.extension.type";
> why do we need type?
type holds the class name of the extension.
Line 34: 
Line 35:     public class ExtensionEntry {
Line 36:         private File file;
Line 37:         private boolean enabled;


Line 65:                     loadedModules.put(moduleSpec, module);
Line 66:                     ServiceLoader<? extends Extension> serviceLoader =
Line 67:                             ServiceLoader.load(Extension.class, 
module.getClassLoader());
Line 68:                     for (Extension extension : serviceLoader) {
Line 69:                         
extensionsByType.put(extension.getClass().getName(), extension.getClass());
> I do not want to load all.... I want to load only what was requested by us.
The motivation behind this is as follows -
we might have the same module for X extensions.
Why perform the loop X times? perform it once, populate the map.
Line 70:                     }
Line 71:                 } catch (ModuleLoadException exception) {
Line 72:                     throw new 
ConfigurationException(String.format("The module '%1$s' cannot be loaded.", 
moduleSpec),
Line 73:                             exception);


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