Hi Seb,
On 16/04/2010 06:20, Sebastien Bahloul wrote:
Hi guys,
You will find there a big patch to add a scripting destination service.
This is very interesting. I've just read through it all, and here are
some comments:
I note that you placed the SimpleScriptingDstService class in the
org.lsc.scripting package. If we consider that the number of services
may grow strongly in the future, this is not an extensible naming
scheme. I propose to add a hierarchy of packages under
"org.lsc.services", for each type of service, including a sub-package
for a services specific classes, if any. What do you think?
Regarding configuration, I see that you use subkeys of
lsc.tasks.ldap2scriptTestTask.dstService to pass both script names and
parameters. I think it would be clearer, and safer, to have two
sub-levels, like:
lsc.tasks.ldap2scriptTestTask.dstService.scripts.list = ...
lsc.tasks.ldap2scriptTestTask.dstService.scripts.add = ...
lsc.tasks.ldap2scriptTestTask.dstService.vars.MY_ENV_VAR = ...
The output from scripts, on stderr, seems to be displayed on a DEBUG log
level only. Maybe we could implement a convention for scripts to display
messages at different levels by prefixing messages on stderr?
I don't understand why you simplify the method
getFullDistinguishedName() from LscBean like this. Are you sure that the
distingushedName local attribute is complete now? It didn't use to be,
but things may have changed since revision 179 :)
At this time, it is doing exactly the same thing that ldap2ldap does through
the provided scripts, but then you can add everything you want inside the 6
scripts (list, get, add, update, rename, delete) to provision external systems
through simple commands
I understand. I see no reason for this to be limited to scripts though -
any executable that can read stdin and output through stdout and stderr
will work :)
It may also contains some stuff about JMX and asynchronous mode that have not
been commited at this, but this is not the important part.
Yes, indeed. This patch is quite hard to read, since it mixes in changes
about a "Task" object, JMX, comment reformatting, etc... Please don't
commit as is, it would be a *nightmare* to track changes, issues, etc! :)
Thank you for taking some time to look at it, at least to check for :
- the IDestinationService to introduce different destination services
- in the SimpleScriptingDestinationService, for the way the scripts are
called.
I see that IDestinationService defines apply(JndiModifications).
Therefore, it is a JNDI-only destination interface, and I suggest it be
renamed IJndiDestinationService.
In the end, we should have our own internal format to store
modifications for a general purpose destination service.
Similarly, SimpleScriptingDestinationService should be
SimpleScriptingJndiDestinationService or maybe just
ScriptingJndiDestinationService ?
To explain it in a short way, scripts are called with :
- environment variables which contains parameters passed in lsc.properties,
- a optional single parameter which is the object main identifier (dn),
- a stdin stream on which the LDIF is written by the service (update ldif
format for update scripts, LDIF search entry format for get)
- a stdout stream which is read for data
- a stderr stream which is read for messages
- a return code
I like the idea of good old standard interaction with executables:
std{in,out,err}, environment variables and return codes. It'll probably
be required to have more command line options at some point, but this
looks good for now.
In fact, if one puts some command line options in the configuration, are
these arguments correctly passed? Like this:
lsc.tasks.ldap2scriptTestTask.dstService.scripts.list = ldapsearch -x -b
o=test ...
Using LDIF only for communication may become limiting at some point
though... It seems a bit weird to me to have to implement a set of shell
scripts that read LDIF to write to a database, for example.
I don't currently have any better suggestions for the format to use, though.
At this time the scripts are only provided as Unix Shell scripts that depend
upon ldapsearch/add/modify/... tools. Ldap2Ldap tests have been duplicated to
check for completeness and are fully fonctional. To complete this
implementation, it will require to write some .bat / .ps1 scripts and to put
some checks inside the Unix implementation.
Well, if I understand correctly, these shell scripts are just for test
purposes? So, maintaining two sets of scripts (Unix shell and .bat/.psl)
sounds like a lot of complicated work for just tests.
Why not implement these "scripts" in some platform-independant language?
We could easily write some simple Java tools that reproduce the
behaviour described, and would work on all platforms. What do you think?
Of course, all this needs documenting too. In this regard, I suggest we
add a hidden section to the wiki, like doc/future/ so we can add new
temporary docs here, if necessary.
This is quite a long email, I hope I've been clear. Please ask if I'm not!
Jonathan
--
--------------------------------------------------------------
Jonathan Clarke - [email protected]
--------------------------------------------------------------
Ldap Synchronization Connector (LSC) - http://lsc-project.org
--------------------------------------------------------------
_______________________________________________________________
Ldap Synchronization Connector (LSC) - http://lsc-project.org
lsc-dev mailing list
[email protected]
http://lists.lsc-project.org/listinfo/lsc-dev
