Alon Bar-Lev has posted comments on this change.

Change subject: [WIP] Introduce new directory interface
......................................................................


Patch Set 1: (5 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/directory/DirectoryManager.java
Line 55:     /**
Line 56:      * Loads the directory implementations that are available in the 
class path
Line 57:      * using the service loader mechanism.
Line 58:      */
Line 59:     private void loadImplementations() {
but I want to have all jars within the classpath per default.

only based on configuration to load a provider with a configuration.

moving files around will not good for packaging... as I cannot install all 
required files and configure later.

or maybe I do not understand this loop.
Line 60:         ServiceLoader<DirectorySpi> loader =
Line 61:             ServiceLoader.load(DirectorySpi.class);
Line 62:         for (DirectorySpi implementation : loader) {
Line 63:             implementations.add(implementation);


....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/users/Directory.java
Line 26:      * @param name the name of the user
Line 27:      * @return the user corresponding to the given name or
Line 28:      *   <code>null</code> if no such user can be found
Line 29:      */
Line 30:     DirectoryUser findUserByName(String name);
Hi,

all searches should be based on queries (within the generic provider anyway)...

and I do think that in order to add user or remove user we might allow 
different searches, non of them generic but a set of attributes we do support.
Line 31: 
Line 32:     /**
Line 33:      * Retrieves an user from the directory given its identifier. Note 
that this
Line 34:      * identifier is the one assigned by the engine to the user, not 
the one


Line 50:      *   with no particular order, note that the returned set may 
contain less
Line 51:      *   elements than the given set of identifiers and that it may be 
in a
Line 52:      *   different order
Line 53:      */
Line 54:     Set<DirectoryUser> findUsersById(Set<Guid> ids);
but these strings will enable us to provide any implementation including local 
repository of uids.

please do not design this for ldap nor to specific directory implementations.
Line 55: 
Line 56:     /**
Line 57:      * Retrieves a group from the directory given its identifier. Note 
that this
Line 58:      * identifier is the one assigned by the engine to the group, not 
the one


Line 80:      *
Line 81:      * @param query the query
Line 82:      * @return a list containing the groups that match the given query
Line 83:      */
Line 84:     Set<DirectoryGroup> findGroupsByQuery(String query);
so we invent a new query language? is it good for us? maybe better to adopt 
some standard directory query language (such as ldap)?


....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/users/DirectoryUser.java
Line 15:     private String firstName;
Line 16:     private String lastName;
Line 17:     private String title;
Line 18:     private String email;
Line 19:     private String department;
it will enable much simpler implementation... why additional 160 bytes in heap 
should make implementation more complex?
Line 20: 
Line 21:     // Flag indicating if this user has the administrator role:
Line 22:     private boolean isAdmin = false;
Line 23: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If84a0c9d6553d81cdbbe224972696f169cca90d4
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to