Alon Bar-Lev has posted comments on this change. Change subject: aaa: use new api ......................................................................
Patch Set 8: (5 comments) http://gerrit.ovirt.org/#/c/26427/8/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticationProfileRepository.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticationProfileRepository.java: Line 81: Line 82: for (ExtensionProxy authnExtension : ExtensionsManager.getInstance().getProvidedExtensions(AUTHN_SERVICE)) { Line 83: registerProfile( Line 84: new AuthenticationProfile( Line 85: (Authenticator) (Extension) authnExtension, > Why this double cast? Can't getExtensionByName return an Extension and have so it will compile. as it won't work now.... must be changed to delegate via invoke. Line 86: (Directory) (Extension) ExtensionsManager.getInstance().getExtensionByName( Line 87: authnExtension.getContext().<Properties>get(Base.ContextKeys.CONFIGURATION).getProperty( Line 88: AUTHN_AUTHZ_PLUGIN Line 89: ) http://gerrit.ovirt.org/#/c/26427/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitBackendServicesOnStartupBean.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitBackendServicesOnStartupBean.java: Line 114: authConfig.put(Base.ConfigKeys.NAME, "builtin-authn-internal"); Line 115: authConfig.put(Base.ConfigKeys.PROVIDES, Authn.class.getName()); Line 116: authConfig.put(Base.ConfigKeys.MODULE, "org.ovirt.engine.extensions.builtin"); Line 117: authConfig.put(Base.ConfigKeys.CLASS, Line 118: "org.ovirt.engine.extensions.aaa.builtin.internal.InternalAuthenticator"); > I know this isn't your fault , but Maybe InternalAuthenticator.class.getNam you cannot do this, as the internal class is not part of the engine. Line 119: authConfig.put("ovirt.engine.aaa.authn.profile.name", "internal"); Line 120: authConfig.put("ovirt.engine.aaa.authn.authz.plugin", "internal"); Line 121: authConfig.put("config.authn.user.name", Config.<String> getValue(ConfigValues.AdminUser)); Line 122: authConfig.put("config.authn.user.password", Config.<String> getValue(ConfigValues.AdminPassword)); Line 124: ExtensionsManager.getInstance().load(authConfig); Line 125: Line 126: Properties dirConfig = new Properties(); Line 127: dirConfig.put(Base.ConfigKeys.NAME, "internal"); Line 128: dirConfig.put(Base.ConfigKeys.PROVIDES, Authz.class.getName()); > here - not sure if I would go for Authz.class.getName - why would you want because it is unique and does not require us to define yet another constant. and we may have dual interface in future, and class is good marker as long as we have. Line 129: dirConfig.put(Base.ConfigKeys.MODULE, "org.ovirt.engine.extensions.builtin"); Line 130: dirConfig.put(Base.ConfigKeys.CLASS, "org.ovirt.engine.extensions.aaa.builtin.internal.InternalDirectory"); Line 131: dirConfig.put("config.authz.user.name", Config.<String> getValue(ConfigValues.AdminUser)); Line 132: ExtensionsManager.getInstance().load(dirConfig); http://gerrit.ovirt.org/#/c/26427/8/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 223: ) Line 224: ); Line 225: Line 226: entry.extension.getContext().put( Line 227: TRACE_LOG_CONTEXT_KEY, > can you please elaborate what this key is for? to hold the trace log, so it can be used with tracing. Line 228: LoggerFactory.getLogger( Line 229: String.format( Line 230: "%1$s.%2$s.%3%s", Line 231: traceLog.getName(), Line 237: Line 238: dumpConfig(entry.extension); Line 239: Line 240: entry.activated = true; Line 241: } catch (Exception ex) { > Would you like to use here the new Exceptions you introduced? or do you int good idea.... will change. Line 242: log.error( Line 243: String.format( Line 244: "Error in activating extension %1$s. Exception message is %2$s", Line 245: entry.name, -- To view, visit http://gerrit.ovirt.org/26427 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7d170d5dda990fd85e9843ecbb4909749a88df75 Gerrit-PatchSet: 8 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
