Yair Zaslavsky has posted comments on this change. Change subject: aaa: Remove dependency at builtin on Common config ......................................................................
Patch Set 3: (3 comments) http://gerrit.ovirt.org/#/c/27607/3/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 213: attachConfigValueFromDb(props, ConfigValues.LDAPOperationTimeout); Line 214: attachConfigValueFromDb(props, ConfigValues.LdapQueryPageSize); Line 215: } Line 216: Line 217: private void attachConfigValueFromDb(Properties props, ConfigValues key) { > I see you pass all in the attachConfigValuesFromDB... just convert it to: ok, now understood. i thought you wanted me to iterate over all literals of ConfigValues. Line 218: props.put("config." + key.name(), Config.getValue(key).toString()); Line 219: } Line 220: Line 221: private String blankIfNull(String value) { http://gerrit.ovirt.org/#/c/27607/3/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/KerberosLdapAuthn.java File backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/KerberosLdapAuthn.java: Line 73: Base.ContextKeys.BUILD_INTERFACE_VERSION, Line 74: Base.INTERFACE_VERSION_CURRENT); Line 75: Line 76: KerberosManager.getInstance(configuration); Line 77: UsersDomainsCacheManagerService.getInstance().init(configuration); > but when someone will come to maintain this he will be confused and do wron i think the singleton is the one of the lesser issues of the code. there is no true separation of authn and authz code in the module and other issues. Line 78: } Line 79: Line 80: Line 81: Line 82: /** Line 83: * {@inheritDoc} Line 84: */ Line 85: private void doAuthenticate(ExtMap input, ExtMap output) { Line 86: input.putIfAbsent(Base.ContextKeys.CONFIGURATION, configuration); > why do you need another? what i meant is that in order to pass the configuration properties to other LdapXXXXCommand I have added another field in the relevant parameters holding the conifugration object. I can do the same here. The important issue is that I have to have the properties of the configuration propagated . Line 87: broker.runAdAction( Line 88: AdActionType.AuthenticateUser, Line 89: new LdapUserPasswordBaseParameters(input, output) Line 90: ); -- To view, visit http://gerrit.ovirt.org/27607 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1384a99f73ab605b61bce8dcdfd63e222b0001fa Gerrit-PatchSet: 3 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
