On 20/07/15 14:07, Oleg Fayans wrote:
Hi Martin,

Updated.


On 07/20/2015 12:46 PM, Martin Basti wrote:
On 20/07/15 11:57, Oleg Fayans wrote:
+        pwfile = api.env.dot_ipa + os.sep + ".dmpw"
+        if ipautil.file_exists(pwfile):
+            fp = open(pwfile, "r")
+            dm_password = fp.read().rstrip()
+            fp.close()
+        else:
Hello,

1) Can you use os.path.join() instead of "+ os.sep +" please

2) Can you use with statement with file?

with open(pwfile, "r") as f:
    dm_password = f.read().rstrip()

3) Please keep PEP8 in new code

./ipatests/test_ipaserver/test_topology_plugin.py:30:80: E501 line too long (102 > 79 characters) ./ipatests/test_ipaserver/test_topology_plugin.py:33:80: E501 line too long (92 > 79 characters) ./ipatests/test_ipaserver/test_topology_plugin.py:39:80: E501 line too long (124 > 79 characters) ./ipatests/test_ipaserver/test_topology_plugin.py:44:80: E501 line too long (92 > 79 characters) ./ipatests/test_ipaserver/test_topology_plugin.py:45:48: E128 continuation line under-indented for visual indent ./ipatests/test_ipaserver/test_topology_plugin.py:45:80: E501 line too long (89 > 79 characters) ./ipatests/test_ipaserver/test_topology_plugin.py:46:48: E128 continuation line under-indented for visual indent ./ipatests/test_ipaserver/test_topology_plugin.py:46:80: E501 line too long (89 > 79 characters) ./ipatests/test_ipaserver/test_topology_plugin.py:58:80: E501 line too long (87 > 79 characters)

4) Missing nose import
raise nose.SkipTest("No directory manager password in %s" % pwfile)

5) Can you use sets here instead of sorted lists?
assert(sorted(entry.keys()) == sorted(pluginattrs.keys()))


Martin^2


1)
Sorry, I didn't notice before, but there is missing header in that file.

2)
You don't need to specify ldap_uri, you just need to call ldap2(api), by default api.env.ldap_uri is used, which is the same as you specified

3)
Can you indent values of dict which are on newline? It is readable better.
            u'nsslapd-topo-plugin-shared-config-base':
                [u'cn=ipa,cn=etc,dc=example,dc=com'],
            u'nsslapd-pluginDescription': [u'ipa-topology-plugin'],

4)
Please use lower F as variable, in python we use capital letters for class definitions
            with open(pwfile, "r") as F:
                dm_password = F.read().rstrip()

Otherwise it works as expected.

Martin^2




--
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

Reply via email to