Re: [389-devel] Please review: various lib389 cleanups

From ff533c70b2b8d31116588aae1c36d457d7cf8697 Mon Sep 17 00:00:00 2001
From: Rich Megginson <rmeggins@redhat.com>
Date: Thu, 21 Nov 2013 09:33:23 -0700
Subject: [PATCH 8/9] move stop and restart to agreement.pause and agreement.unpause

---
lib389/brooker.py | 85 +++++++++++++++++++++++-----------------------------
1 files changed, 38 insertions(+), 47 deletions(-)

diff --git a/lib389/brooker.py b/lib389/brooker.py
index da3d0c2..935f5dc 100644
--- a/lib389/brooker.py
+++ b/lib389/brooker.py
@@ -25,7 +25,8 @@ from lib389._replication import RUV
from lib389._entry import FormatDict

class Agreement(object):
- ALWAYS = None
+ ALWAYS = '0000-2359 0123456'
+ NEVER = '2358-2359 0'

proxied_methods = 'search_s getEntry'.split()

@@ -110,22 +111,18 @@ class Agreement(object):



- def schedule(self, agmtdn, interval='start'):
+ def schedule(self, agmtdn, interval=ALWAYS):
"""Schedule the replication agreement
@param agmtdn - DN of the replica agreement
@param interval - in the form
- - 'ALWAYS'
- - 'NEVER'
+ - Agreement.ALWAYS
+ - Agreement.NEVER
- or 'HHMM-HHMM D+' With D=[0123456]+
@raise ValueError - if interval is not valid
"""

# check the validity of the interval
- if str(interval).lower() == 'start':
- interval = '0000-2359 0123456'
- elif str(interval).lower == 'never':
- interval = '2358-2359 0'
- else:
+ if interval != Agreement.ALWAYS and interval != Agreement.NEVER:
self._check_interval(interval)

# Check if the replica agreement exists
@@ -421,12 +418,42 @@ class Agreement(object):
self.log.info("Starting total init %s" % entry.dn)
mod = [(ldap.MOD_ADD, 'nsds5BeginReplicaRefresh', 'start')]
self.conn.modify_s(entry.dn, mod)
+
+ def pause(self, agmtdn, interval=NEVER):
+ """Pause this replication agreement. This replication agreement
+ will send no more changes. Use the resume() method to "unpause"
+ @param agmtdn - agreement dn
+ @param interval - (default NEVER) replication schedule to use
+ """
+ self.log.info("Pausing replication %s" % agmtdn)
+ mod = [(
+ ldap.MOD_REPLACE, 'nsds5ReplicaEnabled', ['off'])]
+ try:
+ self.conn.modify_s(agmtdn, mod)
+ except LDAPError, e:
+ # before 1.2.11, no support for nsds5ReplicaEnabled
+ # use schedule hack
+ self.schedule(interval)
+
+ def resume(self, agmtdn, interval=ALWAYS):
+ """Resume a paused replication agreement, paused with the "pause" method.
+ @param agmtdn - agreement dn
+ @param interval - (default ALWAYS) replication schedule to use
+ """
+ self.log.info("Resuming replication %s" % agmtdn)
+ mod = [(
+ ldap.MOD_REPLACE, 'nsds5ReplicaEnabled', ['on'])]
+ try:
+ self.conn.modify_s(agmtdn, mod)
+ except LDAPError, e:
+ # before 1.2.11, no support for nsds5ReplicaEnabled
+ # use schedule hack
+ self.schedule(interval)
+


class Replica(object):
proxied_methods = 'search_s getEntry'.split()
- STOP = '2358-2359 0'
- START = '0000-2359 0123456'

def __init__(self, conn):
"""@param conn - a DirSrv instance"""
@@ -593,42 +620,6 @@ class Replica(object):
self.log.info("Setting agreement for continuous replication")
raise NotImplementedError("Check nsds5replicaupdateschedule before writing!")

- def stop(self, agmtdn):
- """Stop replication.
- @param agmtdn - agreement dn
- """
- self.log.info("Stopping replication %s" % agmtdn)
- mod = [(
- ldap.MOD_REPLACE, 'nsds5ReplicaEnabled', ['off'])]
- try:
- self.conn.modify_s(agmtdn, mod)
- except LDAPError, e:
- # before 1.2.11, no support for nsds5ReplicaEnabled
- # use schedule hack
- mod = [(
- ldap.MOD_REPLACE, 'nsds5replicaupdateschedule', [
- Replica.STOP])]
- self.conn.modify_s(agmtdn, mod)
-
- def restart(self, agmtdn, schedule=START):
- """Schedules a new replication.
- @param agmtdn -
- @param schedule - default START
- `schedule` allows to customize the replication instant.
- see 389 documentation for further info
- """
- self.log.info("Restarting replication %s" % agmtdn)
- mod = [(
- ldap.MOD_REPLACE, 'nsds5ReplicaEnabled', ['on'])]
- try:
- self.conn.modify_s(agmtdn, mod)
- except LDAPError, e:
- # before 1.2.11, no support for nsds5ReplicaEnabled
- # use schedule hack
- mod = [(ldap.MOD_REPLACE, 'nsds5replicaupdateschedule', [
- schedule])]
- self.conn.modify_s(agmtdn, mod)
-


def add(self, suffix, binddn, bindpw=None, rtype=REPLICA_RDONLY_TYPE, rid=None, tombstone_purgedelay=None, purgedelay=None, referrals=None, legacy=False):
--
1.7.1

On 11/21/2013 08:36 AM, thierry bordaz wrote:

On 11/21/2013 04:22 PM, Rich Megginson wrote:

On 11/21/2013 01:58 AM, thierry bordaz wrote:

Hi,

The changes look  very good.

I have a question regarding start/stop in Replica class. Why do not you use the function self.agreement.schedule(agmdn, interval='start') and self.agreement.schedule(agmdn, interval='stop') ?

I will change it to use schedule().

about the function 'agreement_dn(basedn, other)' why not putting it into the Agreement class ?
Note that it uses the functions 'agreements' that is Replica but I would expect it to be in Agreement class as well (renamed in 'list' ?).

Yes.  I think I will change the names also, to pause() and resume().

I think the following methods should be moved to the Agreement class: check_init start_and_wait wait_init start_async keep_in_sync agreement_dn (rename to "dn") agreements (rename to "list") (also, is it a problem that we have a method name "list" that is the same as a python keyword/built-in?)

Hi Rich,

Yes that is good. When fixing https://fedorahosted.org/389/ticket/47590 I noticed that with the new Agreement class these functions also needed to be moved. I opened https://fedorahosted.org/389/ticket/47600 for that. 'dn' and 'list' are very good.

When eclipse was complaining with names (function or variable) same as built-in keyword, I tried to change the name. Now I like 'list' name and it was not a problem with self.replica.list(), so I would vote to use 'list'.

New patch attached

regards
thierry

Regards
thierry

On 11/21/2013 03:21 AM, Rich Megginson wrote:

--  389-devel mailing list  389-devel@lists.fedoraproject.org  https://admin.fedoraproject.org/mailman/listinfo/389-devel