[389-devel] Please review ticket 47590 (take #2): add/split functions around replication
Roberto Polli
rpolli at babel.it
Mon Nov 18 10:33:08 UTC 2013
On Monday 18 November 2013 11:08:01 thierry bordaz wrote:
> Hi, this is a second review to take into account the recommendation of
> the first review.
> Major changes are:
>
> * Create a replicaagreement class in brooker
> * mv init/status/schedule/create in that new class
> * mv createDefaultReplMgr into the brooker replica class
> * Handling of error condition with exceptions
>
> https://fedorahosted.org/389/attachment/ticket/47590/0002-Ticket-47590-CI-te
> sts-add-split-functions-around-rep.patch
1- Here and elsewhere:
if not suffix:
# This is a mandatory parameter of the command... it fails
log.warning("createAgreement: suffix is missing")
- return None
+ raise ValueError("suffix is a required value")
2- The method "enableReplication" should probably go into the Replica class,
and be called like
conn.replica.enable()
3- enableReplication and agreementInit should raise in case of errors, not
return 1 or -1. Managing error codes is extra work: and moreover which value
is fine? Greater than zero? Zero? True?
4- If you put replica_createReplMgr in the Replica brooker, just name it
create_manager() so that we can call it
conn.replica.create_manager(). The replica is inferred from the context!
5- If you move the agreement stuff outside Replica, I would just name the
class "Agreement" and set
self.agreement = Agreement(self)
or
self.replica_agreement
--
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.
More information about the 389-devel
mailing list