Thank you for patch, I have a few nitpicks:
1)
On 10/08/15 13:05, Oleg Fayans wrote:
+def create_segment(master, leftnode, rightnode):
+ """create_segment(master, leftnode, rightnode)
Why do you add the name of method in docstring?
2)
+def create_segment(master, leftnode, rightnode):
+ """create_segment(master, leftnode, rightnode)
+ creates a topology segment. The first argument is a node to run the
command on
+ The first 3 arguments should be objects of class Host
+ Returns a hash object containing segment's name, leftnode, rightnode
information
+ """
I would prefer to add assert there instead of just document that a Host object
is needed
assert(isinstance(master, Host))
...
3)
+def destroy_segment(master, segment_name):
+ """
+ destroy_segment(master, segment_name)
+ Destroys topology segment. First argument should be object of class Host
Instead of description of params as first, second etc., you may use
following:
+def destroy_segment(master, segment_name):
+ """
+ Destroys topology segment.
+ :param master: reference to master object of class Host
+ :param segment: name fo segment
and eventually this in other methods
+ :returns: Lorem ipsum sit dolor mit amet
+ :raises NotFound: if segment is not found
4)
cls.replicas[:len(cls.replicas) - 1],
I suggest cls.replicas[:-1]
In [2]: a = [1, 2, 3, 4, 5]
In [3]: a[:-1]
Out[3]: [1, 2, 3, 4]
5)
Why re.findall() and then you just use the first result?
'leftnode': self.leftnode_re.findall(i)[0]
Isn't just re.search() enough?
leftnode_re.search(value).group(1)
Python 3 nitpick:
I'm not sure if time when we should enforce python 2/3 compability already
comes, but just for record:
instead of open(file, 'r'), please use io.open(file, 'r') (import io before)
--
Martin Basti
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
