[Freeipa-devel] [PATCH 0001] Test Topology plugin is listed among DS plugins

[Oleg Fayans]

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

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-ofayans-0001.3-test-topologyplugin-is-listed-among-DS-plugins.patch
Type: text/x-patch
Size: 3560 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150722/00814ecd/attachment.bin>