Alon Bar-Lev has posted comments on this change.

Change subject: aaa: Chages to ExtensionsManager
......................................................................


Patch Set 9:

(3 comments)

http://gerrit.ovirt.org/#/c/27785/9/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 116:     private Map<String, BindingsLoader> bindingsLoaders = new 
HashMap<>();
Line 117:     private Map<String, ExtensionEntry> loadedEntries = new 
HashMap<>();
Line 118:     private ExtMap globalContext = new 
ExtMap().mput(Base.GlobalContextKeys.EXTENSIONS, new ArrayList<ExtMap>());
Line 119: 
Line 120:     private Properties configProperties = new Properties();
so why do we need this now?
Line 121: 
Line 122:     public ExtMap getGlobalContext() {
Line 123:         return globalContext;
Line 124:     }


http://gerrit.ovirt.org/#/c/27785/9/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/extensionsmgr/EngineExtensionsManager.java
File 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/extensionsmgr/EngineExtensionsManager.java:

Line 23:         if (instance == null) {
Line 24:             synchronized (EngineExtensionsManager.class) {
Line 25:                 if (instance == null) {
Line 26:                     instance = new ExtensionsManager(
Line 27:                             
EngineLocalConfig.getInstance().getProperties());
so you can drop now the getProperties()
Line 28:                     
instance.getGlobalContext().put(Base.GlobalContextKeys.APPLICATION_NAME,
Line 29:                             Base.ApplicationNames.OVIRT_ENGINE);
Line 30:                     List<String> extensionNames = new ArrayList<>();
Line 31:                     for (File directory : 
EngineLocalConfig.getInstance().getExtensionsDirectories()) {


Line 51:                                 }
Line 52:                             }
Line 53:                         }
Line 54:                     }
Line 55:                     for (String extensionName : extensionNames) {
oh... why don't you use getExtensions() and drop the extensionNames? this is 
for the tool so that --extension=<properties> will know what it loaded. but 
here you should iterate all.
Line 56:                         if 
(EngineLocalConfig.getInstance().getBoolean(ENGINE_EXTENSION_ENABLED + 
extensionName, true)) {
Line 57:                             instance.activate(extensionName);
Line 58:                         }
Line 59:                     }


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

Reply via email to