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

[Rich Megginson <rmeggins@xxxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/389-devel

From ff533c70b2b8d31116588aae1c36d457d7cf8697 Mon Sep 17 00:00:00 2001
From: Rich Megginson <rmeggins@xxxxxxxxxx>
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

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