Yair Zaslavsky 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... hmm... so how are we going to trace the load? If we can skip EXTENSION_NME in the formatting, of the log name, then we can uset entry.name instead of entry.extension.getContext().get(Base.ContextKeys.INSTANCE_NAME What do you think? 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 o didnt you ask in a previous round to move this to load? i'll put it in initialize 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 as i wrote before, it is not possible to retreive the name of the logger in apache commons logging, at least, not that i've seen. 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? true. 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 Done 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? getters? what getter would you like to set it besides at getExtensionByName? do you want to set it at the getters that return list as well? for example if there are no extensions for a given service? 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.... the problem is our formatter. 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 <[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
