[Engine-patches] Change in ovirt-engine[master]: aaa: Changes to ExtensionsManager

[Alon Bar-Lev]

Alon Bar-Lev has posted comments on this change.

Change subject: aaa: Changes to ExtensionsManager
......................................................................


Patch Set 22:

(7 comments)

http://gerrit.ovirt.org/#/c/27785/22/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 219:                                     getTraceLog(),
Line 220:                                     
entry.extension.getContext().get(Base.ContextKeys.EXTENSION_NAME),
Line 221:                                     
entry.extension.getContext().get(Base.ContextKeys.INSTANCE_NAME)
Line 222:                                     )
Line 223:                             )
this should set after the load as we extension name just after the load...
Line 224:                     );
Line 225: 
Line 226:             ExtMap output = entry.extension.invoke(
Line 227:                     new ExtMap().mput(


Line 253:                             ).mput(
Line 254:                                     Base.ExtensionRecord.CONTEXT,
Line 255:                                     entry.extension.getContext()
Line 256:                             )
Line 257:                     );
the global context should hold only initialized, or we need to add a flag of 
what is initialized.
Line 258:         }
Line 259:         loadedEntries.putIfAbsent(entry.name, entry);
Line 260:         setChanged();
Line 261:         notifyObservers();


Line 263:     }
Line 264: 
Line 265:     public static String getTraceLog() {
Line 266:         return ExtensionsManager.class.getName() + ".trace";
Line 267:     }
I still think that the name should be initialized at deceleration and here you 
return tracelog.getName()
Line 268: 
Line 269:     public ExtMap getGlobalContext() {
Line 270:         return globalContext;
Line 271:     }


Line 272: 
Line 273:     public List<ExtensionProxy> getExtensionsByService(String 
provides) {
Line 274:         List<ExtensionProxy> results = new ArrayList<>();
Line 275:         for (ExtensionEntry entry : initializedEntries.values()) {
Line 276:             if (entry.initialized
of course it is initialized if it is in the initialized entries... now?
Line 277:                     && entry.extension.getContext().<List> 
get(Base.ContextKeys.PROVIDES).contains(provides)) {
Line 278:                 results.add(entry.extension);
Line 279:             }
Line 280:         }


Line 297:         }
Line 298:         return results;
Line 299:     }
Line 300: 
Line 301:     public List<ExtensionProxy> getExtensionss() {
spelling
Line 302:         List<ExtensionProxy> results = new 
ArrayList<>(initializedEntries.size());
Line 303:         for (ExtensionEntry entry : initializedEntries.values()) {
Line 304:             results.add(entry.extension);
Line 305:         }


Line 309:     public ExtensionProxy initialize(String extensionName) {
Line 310:         ExtensionEntry entry = loadedEntries.get(extensionName);
Line 311:         if (entry == null) {
Line 312:             throw new RuntimeException(String.format("No extensioned 
with instance name %1$s could be found",
Line 313:                     extensionName));
so why not have this exception at the getters?
Line 314:         }
Line 315:         try {
Line 316:             ExtMap output = entry.extension.invoke(
Line 317:                     new ExtMap().mput(


http://gerrit.ovirt.org/#/c/27785/22/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 69:                                 Boolean.parseBoolean(
Line 70:                                         
extension.getContext().<Properties>get(Base.ContextKeys.CONFIGURATION).getProperty(Base.ConfigKeys.ENABLED,
 "true")
Line 71:                                         )
Line 72:                                 )) {
Line 73:                             
instance.initialize(extension.getContext().<String> 
get(Base.ContextKeys.INSTANCE_NAME));
ok, now it is ok... but indentation may help....

 if (
     EngineLocalConfig.getInstance().getBoolean(
         (
             ENGINE_EXTENSION_ENABLED +
             extension.getContext().<String> get(Base.ContextKeys.INSTANCE_NAME)
         ),
         Boolean.parseBoolean(
             extension.getContext().<Properties>get(
                 Base.ContextKeys.CONFIGURATION
             ).getProperty(Base.ConfigKeys.ENABLED, "true")
         )
    )
 ) {
         instance.initialize(extension.getContext().<String> 
get(Base.ContextKeys
.INSTANCE_NAME));
 }

notice, each parameter at own line or indented with (). I am sure your IDE 
cannot cope :)
Line 74:                         }
Line 75:                     }
Line 76:                 }
Line 77:             }


-- 
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: 22
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
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