[Freeipa-devel] [PATCH 0001] Test Topology plugin is listed among DS plugins
Oleg Fayans
ofayans at redhat.com
Thu Jul 23 09:42:59 UTC 2015
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
>
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-ofayans-0001.4-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/20150723/6dbf653a/attachment.bin>
More information about the Freeipa-devel
mailing list