Yair Zaslavsky has posted comments on this change.

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


Patch Set 2:

(2 comments)

http://gerrit.ovirt.org/#/c/27607/2/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 136:         dirConfig.put(Base.ConfigKeys.BINDINGS_JBOSSMODULE_CLASS, 
"org.ovirt.engine.extensions.aaa.builtin.internal.InternalAuthz");
Line 137:         dirConfig.put("config.authz.user.name", Config.<String> 
getValue(ConfigValues.AdminUser));
Line 138:         dirConfig.put("config.authz.user.id", 
"fdfc627c-d875-11e0-90f0-83df133b58cc");
Line 139:         dirConfig.put("config.query.filter.size",
Line 140:                 Config.<Integer> 
getValue(ConfigValues.MaxLDAPQueryPartsNumber).toString());
> you should use putProperty instead of put to avoid these
ok.
Line 141:         ExtensionsManager.getInstance().load(dirConfig);
Line 142:     }
Line 143: 
Line 144:     private void createKerberosLdapAAAConfigurations() {


Line 180:                 
authConfig.put(Base.ConfigKeys.BINDINGS_JBOSSMODULE_CLASS,
Line 181:                         
"org.ovirt.engine.extensions.aaa.builtin.kerberosldap.KerberosLdapAuthn");
Line 182:                 authConfig.put("ovirt.engine.aaa.authn.profile.name", 
domain);
Line 183:                 authConfig.put("ovirt.engine.aaa.authn.authz.plugin", 
domain);
Line 184:                 authConfig.put("config", config);
> why do you put properties within properties? it will make it very hard to c
Please see the class Config at the builtin extension.
The values read from vdc_options are "shared' in their nature between all 
extensions, so I wanted to create a single object containing them, and pass it 
to the relevant extensions configurations, and then let Config (at builtin 
extension) use this object.
However, I understand your comment.
What I suggest is to have a special prefix -
"config.shared" That will mark the shared configuration.
I would prefer to distinguish between "shared" configuration and non shared if 
possible.
Line 185:                 ExtensionsManager.getInstance().load(authConfig);
Line 186: 
Line 187:                 Properties dirConfig = new Properties();
Line 188:                 dirConfig.put(Base.ConfigKeys.NAME, domain);


-- 
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: 2
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