On 23/07/15 16:27, Martin Basti wrote:
On 23/07/15 11:42, Oleg Fayans wrote:
Forgot to attach the new version, sorry!
On 07/23/2015 10:32 AM, Oleg Fayans wrote:
Hi Martin,
On 07/22/2015 05:48 PM, Martin Basti wrote:
On 22/07/15 15:19, Oleg Fayans wrote:
Hi Martin,
Fixed.
On 07/22/2015 09:26 AM, Martin Basti wrote:
On 22/07/15 09:23, Oleg Fayans wrote:
Hi Martin,
Patch updated. Thank you for the review!
On 07/21/2015 05:45 PM, Martin Basti wrote:
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
Sorry.
You added there old license format, we now use in new files new
format
#
# Copyright (C) 2015 FreeIPA Contributors see COPYING for license
#
I cannot apply the last patch
$ git am
freeipa-ofayans-0001.3-test-topologyplugin-is-listed-among-DS-plugins.patch
-3
Applying: Added test - topology plugin is listed among DS plugins
fatal: corrupt patch at line 83
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Fixed. Tested it locally, it applies
Thank you ACK
--
Martin Basti
Pushed to master: e5acd01ed2971be779e788937493844a9926bb96
--
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
