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
https://fedorahosted.org/389/attachment/ticket/47628/0001-Ticket-47628-port-...
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-port-... testcases-to-new-DirSrv-interface.patch
Hello Roberto,
Thanks, I also wish you an happy new year and you recover well. It is great to talk to you again :-) .
I am quite novice with python and my first approach was to use it as a real object oriented language, having instances of everything (suffix, MT, replica, indexes...). Then discussing with Jan and looking at your implementation of dsadmin, I realized that 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(..) ') The big advantage I see in functional approach is that the 'object' are no long python objects but the configuration object stored in 'cn=config'. So it prevents the problem of synchronizing the python object with the 'cn=config' object. 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 ?
About the shift of parameters to dictionary I think both approaches are valid. 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 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 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-port-... testcases-to-new-DirSrv-interface.patch
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
On 01/08/2014 12:54 PM, Roberto Polli wrote:
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
Hi Roberto,
I will try to answer your concerns but as all of them are valid I will only give some reasons for the approach I choose.
About OOL vs. functional programing this is not IMH that important. For example for an instance with N replicas, we can imagine DSAdmin having N Replica/Backend/Suffix/MappingTree... objects. Instead we have only one, let's say Replica object, allocated in __add_brookers__ but this object is not a real object but just gives access all methods of Replica class. As I said, having N Replica objects brings a big drawback to synchronize the objects with what is in the server config. So I think the __add_brookers__ approach is better.
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)
I agree with you DSAdmin approach looks definitely simpler. Now there is some magic behind DSAdmin(). It actually checks if the instance exists, if not it creates it, then it opens a connection to it. What I wanted to do in a lib is to give an interface to each individual action. We may want to create an instance without establishing a connection to it, or (re)create an instance even if one already exists. Your point is valid, we need something simple. So what about adding a new interface to DirSrv (e.g. NewAndConnect) that would do all the steps 1-5 ?
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.
DirSrv also extends SimpleLDAPObject and wrap all its methods (search/add...) what would prevent it to be use as mock ldap ?
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.
ok
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.
Yes that looks very smart. Couldn't it been done with DirSrv that in my understanding is similar to DSAdmin ? (except specific interface to create/open).
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).
Is your concern (parameters vs dictionary) specific to DirSrv/DSAdmin class ? Have you the same concern for others classes (Replica, Backend, MT, Agreement..) ?
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;
I agree with all your points. Now the status is not that bad with dictionary when you are using an IDE. It shows the header comment of the function you keyed and that header can contain the args list and explanation.
Note that an benefit of dictionaries (to provide the properties/attribute) is that it standardizes the interface for all classes. Any create/set-prop methods of any class takes a set of of properties through a dictionary. With args the interface will vary a lot from let's say MT that have few attributes to RA/DirSrv that have a lot of attributes.
About the default value, yes parameters brings an advantage. Now this is an advantage for lib389 implementation not for lib389 user.
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)
Yes 'allocate' is then similar to 'NewAndConnect' I said before.
Let me know. Regards thierry
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
Hi Thierry + @all,
I'd like to play with the new lib389 and try to split DirSrv in two layers: - the "old approach" DSAdmin for TCP communication - DirSrv implementing your interface
essentially I would put class DirSrv(DSAdmin): # ...new stuff go here ...
class DSAdmin(SimpleLDAPObject): # TCP stuff goes here
Can you please tell me: 1- how to run tests 2- which tests should work 3- which is the test environment.
Thx+Peace, R.
On Thursday 09 January 2014 10:29:10 thierry bordaz wrote:
On 01/08/2014 12:54 PM, Roberto Polli wrote:
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
Hi Roberto,
I will try to answer your concerns but as all of them are valid I will only give some reasons for the approach I choose. About OOL vs. functional programing this is not IMH that important. For example for an instance with N replicas, we can imagine DSAdmin having N Replica/Backend/Suffix/MappingTree... objects. Instead we have only one, let's say Replica object, allocated in __add_brookers__ but this object is not a real object but just gives access all methods of Replica class. As I said, having N Replica objects brings a big drawback to synchronize the objects with what is in the server config. So I think the __add_brookers__ approach is better.
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)
I agree with you DSAdmin approach looks definitely simpler. Now there is some magic behind DSAdmin(). It actually checks if the instance exists, if not it creates it, then it opens a connection to it. What I wanted to do in a lib is to give an interface to each individual action. We may want to create an instance without establishing a connection to it, or (re)create an instance even if one already exists. Your point is valid, we need something simple. So what about adding a new interface to DirSrv (e.g. NewAndConnect) that would do all the steps 1-5 ?
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.
DirSrv also extends SimpleLDAPObject and wrap all its methods (search/add...) what would prevent it to be use as mock ldap ?
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.
ok
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.
Yes that looks very smart. Couldn't it been done with DirSrv that in my understanding is similar to DSAdmin ? (except specific interface to create/open).
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).
Is your concern (parameters vs dictionary) specific to DirSrv/DSAdmin class ? Have you the same concern for others classes (Replica, Backend, MT, Agreement..) ?
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;
I agree with all your points. Now the status is not that bad with dictionary when you are using an IDE. It shows the header comment of the function you keyed and that header can contain the args list and explanation. Note that an benefit of dictionaries (to provide the properties/attribute) is that it standardizes the interface for all classes. Any create/set-prop methods of any class takes a set of of properties through a dictionary. With args the interface will vary a lot from let's say MT that have few attributes to RA/DirSrv that have a lot of attributes. About the default value, yes parameters brings an advantage. Now this is an advantage for lib389 implementation not for lib389 user.
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)
Yes 'allocate' is then similar to 'NewAndConnect' I said before. Let me know. Regards thierry
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-%3E >>>> po rt-> testcases-to-new-DirSrv-interface.patch
On 01/23/2014 12:42 PM, Roberto Polli wrote:
Hi Thierry + @all,
I'd like to play with the new lib389 and try to split DirSrv in two layers:
- the "old approach" DSAdmin for TCP communication
- DirSrv implementing your interface
essentially I would put class DirSrv(DSAdmin): # ...new stuff go here ...
class DSAdmin(SimpleLDAPObject): # TCP stuff goes here
Can you please tell me: 1- how to run tests
In a day to day work, I prefer to run an individual test under eclipse so that I can isolate the tests.
I recommend you look at http://port389.org/wiki/Upstream_test_framework#Deploy_389-ds_under_specific... then at http://port389.org/wiki/Upstream_test_framework#Run_a_specific_test_under_ec...
2- which tests should work
You may run dirsrv_test.py, backend_test.py, mappingTree_test.py, replica_test.py and Suffix_test.py.
3- which is the test environment.
Currently the tests are run only under eclipse (or directly with python but require to set PYTHONPATH). ticket fix have their own tests on top of lib389. Those tests are run under jenkins/py.test
Thx+Peace, R.
On Thursday 09 January 2014 10:29:10 thierry bordaz wrote:
On 01/08/2014 12:54 PM, Roberto Polli wrote:
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
Hi Roberto,
I will try to answer your concerns but as all of them are valid I will only give some reasons for the approach I choose. About OOL vs. functional programing this is not IMH that important. For example for an instance with N replicas, we can imagine DSAdmin having N Replica/Backend/Suffix/MappingTree... objects. Instead we have only one, let's say Replica object, allocated in __add_brookers__ but this object is not a real object but just gives access all methods of Replica class. As I said, having N Replica objects brings a big drawback to synchronize the objects with what is in the server config. So I think the __add_brookers__ approach is better.
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)
I agree with you DSAdmin approach looks definitely simpler. Now there is some magic behind DSAdmin(). It actually checks if the instance exists, if not it creates it, then it opens a connection to it. What I wanted to do in a lib is to give an interface to each individual action. We may want to create an instance without establishing a connection to it, or (re)create an instance even if one already exists. Your point is valid, we need something simple. So what about adding a new interface to DirSrv (e.g. NewAndConnect) that would do all the steps 1-5 ?
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.
DirSrv also extends SimpleLDAPObject and wrap all its methods (search/add...) what would prevent it to be use as mock ldap ?
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.
ok
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.
Yes that looks very smart. Couldn't it been done with DirSrv that in my understanding is similar to DSAdmin ? (except specific interface to create/open).
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).
Is your concern (parameters vs dictionary) specific to DirSrv/DSAdmin class ? Have you the same concern for others classes (Replica, Backend, MT, Agreement..) ?
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;
I agree with all your points. Now the status is not that bad with dictionary when you are using an IDE. It shows the header comment of the function you keyed and that header can contain the args list and explanation. Note that an benefit of dictionaries (to provide the properties/attribute) is that it standardizes the interface for all classes. Any create/set-prop methods of any class takes a set of of properties through a dictionary. With args the interface will vary a lot from let's say MT that have few attributes to RA/DirSrv that have a lot of attributes. About the default value, yes parameters brings an advantage. Now this is an advantage for lib389 implementation not for lib389 user.
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)
Yes 'allocate' is then similar to 'NewAndConnect' I said before. Let me know. Regards thierry
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-%3E >>>> po rt-> testcases-to-new-DirSrv-interface.patch
Hi T,
On Thursday 23 January 2014 14:19:52 thierry bordaz wrote:
In a day to day work, I prefer to run an individual test under eclipse so that I can isolate the tests.
I recommend you look at http://port389.org/wiki/Upstream_test_framework#Deploy_389-ds_under_specific _directory
done, I sent a mail about how "fixing" some scripts
http://port389.org/wiki/Upstream_test_framework#Run_a_specific_test_under_ec lipse
with some small changes I was able to run the test under both nose and py.test. Using Eclipse + Nose you could even run single tests with a right- click on the test name.
mappingTree_test.py,
the "list_test" depends on "create_test" and can't be executed standalone. I would implement those tests separately. I would even simplify the tests, as they are quite long and complex.
You may run dirsrv_test.py, backend_test.py, replica_test.py and Suffix_test.py.
I'll check them
Currently the tests are run only under eclipse (or directly with python but require to set PYTHONPATH).
ok
ticket fix have their own tests on top of lib389. Those tests are run under jenkins/py.test
ok.
See the attached png for the eclipse settings...
Peace, R.
On 01/24/2014 03:50 PM, Roberto Polli wrote:
Hi T,
On Thursday 23 January 2014 14:19:52 thierry bordaz wrote:
In a day to day work, I prefer to run an individual test under eclipse so that I can isolate the tests.
I recommend you look at http://port389.org/wiki/Upstream_test_framework#Deploy_389-ds_under_specific _directory
done, I sent a mail about how "fixing" some scripts
Yes. My only concern is to keep the ability to run the test directly under python. Do we really need to get rid of __main__ when using nose/py.test ?
http://port389.org/wiki/Upstream_test_framework#Run_a_specific_test_under_ec lipse
with some small changes I was able to run the test under both nose and py.test. Using Eclipse + Nose you could even run single tests with a right- click on the test name.
Great !
mappingTree_test.py,
the "list_test" depends on "create_test" and can't be executed standalone. I would implement those tests separately. I would even simplify the tests, as they are quite long and complex.
Ok, but I do not understand how you can test 'list' if you have not 'create' mapping_tree before.
You may run dirsrv_test.py, backend_test.py, replica_test.py and Suffix_test.py.
I'll check them
Currently the tests are run only under eclipse (or directly with python but require to set PYTHONPATH).
ok
ticket fix have their own tests on top of lib389. Those tests are run under jenkins/py.test
ok.
See the attached png for the eclipse settings...
Peace, R.
On Friday 24 January 2014 15:56:10 thierry bordaz wrote:
Do we really need to get rid of __main__ when using nose/py.test ?
Obviously you can take it. I would just avoid noise in testing.
Moreover they could leave your environment dirty: eg. if a test fails it doesn't run the teardown function.
I do not understand how you can test 'list' if you have not 'create' mapping_tree before.
I would have created the list_test requirements in the setup phase. (in the link below I created two different backends for two different test cases) https://github.com/ioggstream/dsadmin/blob/merge_lib389/tests/replica_test.p...
While I started with dependent tests on dsadmin, I spent some time to change that approach because small changes caused whole testsuites to fail and it was difficult to find where the error was (in the setup? in the test first code? in the second test code? in the library code? )
Peace, R.
On 01/24/2014 04:18 PM, Roberto Polli wrote:
On Friday 24 January 2014 15:56:10 thierry bordaz wrote:
Do we really need to get rid of __main__ when using nose/py.test ?
Obviously you can take it. I would just avoid noise in testing.
Moreover they could leave your environment dirty: eg. if a test fails it doesn't run the teardown function.
I do not understand how you can test 'list' if you have not 'create' mapping_tree before.
I would have created the list_test requirements in the setup phase. (in the link below I created two different backends for two different test cases) https://github.com/ioggstream/dsadmin/blob/merge_lib389/tests/replica_test.p...
While I started with dependent tests on dsadmin, I spent some time to change that approach because small changes caused whole testsuites to fail and it was difficult to find where the error was (in the setup? in the test first code? in the second test code? in the library code? )
Ok I agree
Peace, R.
389-devel@lists.fedoraproject.org