Alon Bar-Lev has posted comments on this change.

Change subject: aaa: using the new extensions API in InternalDirectory
......................................................................


Patch Set 4:

(3 comments)

http://gerrit.ovirt.org/#/c/26477/4/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/DirectoryProxy.java
File 
backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/DirectoryProxy.java:

Line 59:             return populateUsers(inputMap);
Line 60:         } finally {
Line 61:             extension.invoke(new ExtMap().mput(
Line 62:                     Base.InvokeKeys.COMMAND, 
Authz.InvokeCommands.QUERY_CLOSE
Line 63:                     ).mput(Authz.InvokeKeys.QUERY_OPAQUE, 
queryOpaque));
> but i did invoke open - QUERY_RINCIPALS_BY_IDS_OPEN
where have you actually invoke? call the invoke()  with 
QUERY_RINCIPALS_BY_IDS_OPEN?
Line 64:         }
Line 65:     }
Line 66: 
Line 67:     @Deprecated


Line 109:         }
Line 110:         inputMap = new ExtMap().mput(
Line 111:                 Base.InvokeKeys.COMMAND, 
Authz.InvokeCommands.QUERY_CLOSE
Line 112:                 ).mput(Authz.InvokeKeys.QUERY_OPAQUE, queryOpaque);
Line 113:         outputMap = extension.invoke(inputMap);
> actually due to the finally block, this QUERY_CLOSE invocation should be re
I do not follow...
Line 114:         return directoryUsers;
Line 115:     }
Line 116: 
Line 117:     private List<DirectoryGroup> populateGroups(ExtMap inputMap) {


Line 135:                 Base.InvokeKeys.COMMAND, 
Authz.InvokeCommands.QUERY_CLOSE
Line 136:                 ).mput(Authz.InvokeKeys.QUERY_OPAQUE, queryOpaque);
Line 137:         outputMap = extension.invoke(inputMap);
Line 138:         return directoryGroups;
Line 139:     }
> i don't see such a great benefit to it here.
so you prefer to duplicate the exact code in both populate? and in future in 
more...?

I prefer one logic to perform the iteration and focus on logic outside.

but we will see...
Line 140: 
Line 141:     @Deprecated
Line 142:     @Override
Line 143:     public List<DirectoryGroup> queryGroups(String query) {


-- 
To view, visit http://gerrit.ovirt.org/26477
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I958443292da0455e0a12039fac98eebb9b17dee2
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to