Hi T,
sorry for the laconicism but that's the only way to contribute today.
On Thursday 14 November 2013 22:23:58 thierry bordaz wrote:
I agree that 'setup' is commonly used to prepare/initialize
a
functionality. Now I would prefer verbs of action/unaction like
create/delete, enable/disable, set/get/list. With 'setup' verb we may
create entries, enable functionality, set properties. If we have a
function setupAgreement (that creates the RA and enables it by default)
what is the name for the function that delete the agreement
'deleteAgreement' ?
As of now we can still use these names, but the essence
is: put them in the
"Tools" part and we'll find the right way to use them.
So far DSAdmintools mainly contains offline functions (like
start/stop
instance). I agree we can put setup functions in it. But I wonder if it
would be interesting to keep all offline functions in a separated file.
Thanks to
inheritance, merging two class is free, splitting not. If tomorrow
we decide to merge Tools and Admin we just have to mix-in DsAdminTool.
http://en.wikipedia.org/wiki/Mixin#In_Python
> 2- methods like _createDefaultReplMgr are not expected to be
used in
> production, so should be placed in the DSAdminTools section
I am not sure. If someone wants to rapidly deploy a replication
topology, he would be interested to have a default replication manager.
The idea I
put under dsadmin is to remove everything that is not essential:
less is more (so we have even less base code to test).
People should have:
* as few methods as possibile;
* methods should be explicative and deterministic;
* should be able to remeber them;
* should be able to find them using self-completion;
One issue we had with the old Rich library is that it put all the methods at
the root of the class, so whatever you were trying to do you had too many
choiches. Moreover the rationale of the behavior was unclear because methods
tried to solve automatically as many errors or cases they could... to be clear
they were too intelligent for the unacquainted user.
In that case we may offer 'createDefaultReplMgr' (without
heading '_').
In your opinion what kind of functions would go into DSAdminTools ? all
'setupxxx' functions ?
I would put into DSAdminTools whatever method is not
enough general to be used
in a standard use case. Moreover consider that complex methods - if exposed -
should be tested with almost all the input/output. Making complex setup
methods makes it quite impossible.
Eg. it is fine to decide a default replica type, not fine to put user
credentials. To clarify I'm even against using a default database file name
for backends.
> 3- the brooker naming convention is based on the
"function-first" so that
> python interactive users can tab and autocomplete it.
>
> initAgreement should be renamed to something like agreement_init or
> something>
> else. For the return codes, simply use exceptions.
ok. So do you prefer names like 'replica_create', 'suffix_create',
'agreement_create', rather than 'createReplica', 'createSuffix',
'createAgreement' ? Ok I will change the name.
Right. If you started using
ipython and its self-completion stuff you'll
easily understand the gain of that approach!
It may be worth checking the differences between the original dsadmin code
used for bugfix and the latest.
Sorry again for the short time I had to write you this mail :(
Peace,
R.
> = exception handling & None return =
> 1- in case of errors, a method should raise a proper exception and
> eventually log the error
> 2- so the assert clauses should be replaced by exception because they mean
> that something went wrong and an action should be taken
> 3- about the bindmethod stuff: see
http://pastebin.com/w0WnVQuJ
Absolutely, I will change the error handling. In addition, exception
makes most of the time the code easier to read.
Thanks
I will resend a review according to you suggestions.
regards
thierry
> There are some other points but the best way to set them is with patches.
>
> Let me know + Peace,
> R.
>
> On Wednesday 13 November 2013 17:06:25 thierry bordaz wrote:
>> In order to implement the first CI test with replication instances, I
>> reorganised lib389 functions related to replication setup.
>>
>>
https://fedorahosted.org/389/attachment/ticket/47590/0001-Ticket-47590-CI
>> -te sts-add-split-functions-around-rep.patch
--
Roberto Polli
Community Manager
Babel S.r.l. -
http://www.babel.it
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.