Hi Thierry,
before all sorry if in the rest of the answer I may seem too direct. All I say
is obviously imh(opinion|experience) Time is a tyrant :(
On Friday 03 January 2014 16:36:17 thierry bordaz wrote:
> Thanks, I also wish you an happy new year and you recover well. It
> is great to talk to you again :-) .
Thx++!
> I am quite novice with python and my first approach was to use it as
> a real object oriented language,
It *is* a real OOL ;)
> it is a
> common use to wrap methods of class and rather use python as a
> functional language (e.g. self.backend.setProperties(...) rather
> than 'backend=Backend(); backend.setProperties(..) ')
I'm not indepth with functional programming. Why do you think that's a
functional approach?
> ...the 'object'...are... the configuration object stored in
> 'cn=config'. So it prevents the problem of synchronizing the python
> object with the 'cn=config' object.
To me that's mostly an ORM approach: in any case that's ok
> So the only remaining object is
> the DS instance (dirsrv/dsadmin) and methods to access its config.
>
> Does it prevent to use fake directory ? I am not enough familiar
> with fakeldap, would you give me an example of why the current
> implement would not work with fakeldap ?
Let's see how do we setup the client now:
args = {SER_HOST: LOCALHOST,
SER_PORT: INSTANCE_PORT,
SER_DEPLOYED_DIR: INSTANCE_PREFIX,
SER_SERVERID_PROP: INSTANCE_SERVERID
}
1- instance = DirSrv(verbose=False)
2- instance.allocate(args)
3- if instance.exists(): instance.delete()
4- instance.create()
5- instance.open()
That's quite different from the (imho cleaner) old approach - similar to the
SimpleLDAPObject superclass:
args = {'host': LOCALHOST, 'sslport': 10636}
1- instance = DSAdmin(**args)
Obviously there are plenty of functionalities DSAdmin didn't implement: I
would have put those functionalities (eg. filesystem related, instance
creation & removal) in the DSAdminTool class.
You may ask: why having two class DSAdminTool and DSAdmin instead of just one?
1- clear design: as DSAdmin extends SimpleLDAPObject, it should require just a
tcp connection (like SimpleLDAPObject). In this way I can use a mock ldap
implementation to test the LDAP behavior of DSAdmin.
2- all the functionalities requiring filesystem access and instance management
(eg. outside LDAP scope) couldn't be tested by a mock ldap implementation, so
we can just extend the mock for those functionalities.
3- As extending classes (or using mix-ins) in python is really smooth, this
approach would lead to the same usage patterns we have now, but with a
separate, tcp oriented DSAdmin class.
> About the shift of parameters to dictionary I think both approaches
> are valid.
> ... I used a dictionary to pass
> the properties (rather than functions arguments) mainly because some
> resource may have many properties, also because all the
> set-<resource>-prop function have a similar interface.
I understand your choice in the view of the CLI. I just think it shouldn't
influence the layout of a class which extends SimpleLDAPObject (see §3 of the
previous statement).
Here's my rationale of why I stand for the parameter-based (choice made after
a discussion on #python):
1- parameter-based approach is self-documenting;
2- interactive python self-completes parameters, so you can use it
without knowing the whole interface;
3- a parameter-based function can take a **dict input, converse is not
true;
4- parameter-based saves you from managing default values;
5- parameter-based makes clear which are the default values;
The parameter approach can easily be wrapped elsewhere:
class DSAdmin(..):
def __init__(host='localhost', port=389, furtherargs={}):
# doit
class DirSrv():
def allocate(allargs):
# put the following code in an utility function
proxy_params = DSAdmin.__init__.func_code.co_varnames
params = {k:v for k,v in allargs.items() if k in proxy_parameters}
params['furtherargs'] = {k:v for k,v in allargs.items() if k not in
proxy_parameters}
self.instance = DSAdmin(**params)
Moreover the utility function could even be used for mapping the human-
readable values to the internal ones.
Let me know + Peace,
R.
> Just let me explain why I choose dictionary, it comes
> from what could be future CLI.
> for example we may have a CLI pattern like:
>
> <admin_cmd> <verb-resource> [parameters] [options]
>
> admin_cmd::= "dsadmin"
> verb-resource::= <verb-suffix> | <verb-replication> | <verb-backend>
>
> |...
>
> <verb-suffix> ::= list-suffix | create-suffix | delete-suffix...
> parameter::= <parameters to connect to the DS instance (port,
> credential..)>
> options::= <option to process the command and also some properties>
>
> Then we have specific <verb-resource> to define the resource
> properties (entry cache size, directory path, type of replica...)
>
> They are set-suffix-prop, set-backend-prop...
> Basically once the resource has been created (with some properties),
> any 'properties' of the resource can be read/write by
> set/get-<resource>-prop.
>
> So for the CLI (or other admin application) we need to send
> prop:value to an instance. prop are then available in an include
> file, but in a human-readable way (like 'suffix') rather than in an
> attribute way (like 'nsds5replicaroot')
>
> I do not know if I answer correctly your concerns. Just let me know
> if I missed your questions.
>
> regards
> thierry
>
> On 01/03/2014 01:10 PM, Roberto Polli wrote:
> > Hi Everybody!
> >
> > First of all Happy New Year, hope it will be filled of fun!
> >
> > I am really sorry for not supporting you for so long, but as I told you I
> > can't still spend too much extra-work time coding :(.
> >
> > I saw the evolution of the library and I'm glad of the new functionalities
> > and the split-up of the brooker in various files.
> >
> > On the other hand I think that the new interface changes don't go on the
> > simplicity/usability path, so I'd like to understand your targets and give
> > some suggestions.
> >
> > On Thursday 12 December 2013 21:58:12 thierry bordaz wrote:
> >> Ticket https://fedorahosted.org/389/ticket/47625 changes the interface
> >> of DirSrv.
> >> This ticket is the porting side of the test cases to the new interface
> >
> > 1- The new interface initialization is substantially different from a
> > standard>
> > client-server interface. A user expects to instantiate objects like this:
> > # client = Telnet(host, port, credential)
> >
> > 2- This is quite the behavior of the LDAPObject superclass, which I would
> > like to retain so that we can use fakeldaps for unittest
> >
> > 3- The standard DirSrv.__init__ (and the same is valid for other
> > methods),
> > containing a set of parameters is intrinsically documented. Shifting core
> >
> > parameters in dictionaries:
> > a- de-documents parameters and default values outside the method
> > signature;
> > b- requires parsing and setting of default values;
> > Python allows to retain the dict-style stuff using the **magic, which I
> > would>
> > embrace in this case.
> >
> > The new Interface Layer would be easily implemented in a subclass.
> >
> > Let me know + Peace (and sorry again for my absence),
> > R.
> >
> >> https://fedorahosted.org/389/attachment/ticket/47628/0001-Ticket-47628-po
> >> rt-> testcases-to-new-DirSrv-interface.patch
--
Roberto Polli
Community Manager
Babel S.r.l. - http://www.babel.it - una business unit di Par-Tec S.p.A.
T: +39.06.9826.9651 M: +39.340.652.2736 F: +39.06.9826.9680
P.zza S.Benedetto da Norcia, 33 - 00040 Pomezia (Roma)
CONFIDENZIALE: Questo messaggio ed i suoi allegati sono di carattere
confidenziale per i destinatari in indirizzo.
E' vietato l'inoltro non autorizzato a destinatari diversi da quelli indicati
nel messaggio originale.
Se ricevuto per errore, l'uso del contenuto e' proibito; si prega di
comunicarlo al mittente e cancellarlo immediatamente.
--
389-devel mailing list
389-devel@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/389-devel
