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.