Alon Bar-Lev has posted comments on this change.

Change subject: aaa: Remove dependency at builtin on Common config
......................................................................


Patch Set 3:

(3 comments)

at initialize you should add defaults for missing keys within the properties, 
so that you always have values, even if loaded externally.

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) {
you can do ConfigValues... keys and loop.... and call it single time....
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);
this is still singleton?!?!
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);
I do not understand how come you need to pu tconfiguration.
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

Reply via email to