[PR PATCH] Bulk octal and exception fixup changes

[Noel Power]

On 28/02/18 03:41, Douglas Bagnall wrote:

[...]
>> additionally this pull request contains a patches to do a bulk
>> conversion where except clause specifies a tuple following the
>> exception name. This format is illegal in python3 e.g.
>>
>> convert
>>
>>     except LdbError, (num, msg):
>> to
>>     except LdbError as e:
>>          (num, msg) = e.args
> Here there are a few mistakes where a comment ends up being duplicated:
<sigh> I thought I caught all of those :-(
>
>> Subject: [PATCH 09/10] dsdb python tests: convert 'except X, (tuple)' to
>> --- a/source4/dsdb/tests/python/passwords.py
>> +++ b/source4/dsdb/tests/python/passwords.py
>> @@ -340,7 +350,9 @@ def test_clearTextPassword_clear_set(self):
>>                FLAG_MOD_REPLACE, "clearTextPassword")
>>              self.ldb.modify(m)
>>              # this passes against s4
>> -        except LdbError, (num, msg):
>> +        except LdbError as e10:
>> +            # "NO_SUCH_ATTRIBUTE" is returned by Windows -> ignore it
>> +            (num, msg) = e10.args
>>              # "NO_SUCH_ATTRIBUTE" is returned by Windows -> ignore it
>>              if num != ERR_NO_SUCH_ATTRIBUTE:
>>                  raise LdbError(num, msg)
> which I have fixed manually.
>
> I suspect you agree that the proliferation of exception variables
> ("e10" etc, rather than all "e") is unfortunate, but I know it would
> be tricky to ensure all "e" was safe in an automatic process. And it
> is good to get this done, so I won't block it those grounds. But this
> is about the limit: we don't want automated changes whose results are
> more inhuman than this.
yep, agree with all the points above
>
> (What I would *really* like is for LdbError to grow attributes like
> IOError's errno and strerror, so the above could be:
>
>         except LdbError as e:
>             # "NO_SUCH_ATTRIBUTE" is returned by Windows -> ignore it
>             if e.errno != ERR_NO_SUCH_ATTRIBUTE:
>                 raise
>
> but that is not relevant to this patch.)
>
> The last line of the first patch's commit message is slightly worrying:
>
>> Subject: [PATCH 01/10] s4/dsdb: python3 api should take 'bytes'
>>
>> Attributes are properly represented by 'bytes' and *maybe* can be converted
>> into strings (if they are text). py_dsdb_normalise_attributes
>> currently expects strings, this is fine in python2 however in python3 we
>> need to actually pass a 'bytes' class.
>>
>> Signed-off-by: Noel Power <noel.power at suse.com>
>>
>> squash
> Did you mean to submit it in this form?
haha I didn't mean to submit that (or the following) patch in this pull
request rather just on the mailing list instead (which incidentally has
the same stray line) Anyway, I wont object to the inclusion and review
(thankyou!) of these patches. The 'squash' is nothing sinister other
than I didn't see I missed this stray comment when squashing a
correction into this patch in an earlier iteration
>
>> -		if (!PyStr_Check(str)) {
>> -			PyErr_SetString(PyExc_TypeError, "Expected a string argument to GUID()");
>> +		/* in python2 we get a string, in python3 we expect bytes */
>> +		if (!PyBytes_Check(str)) {
>> +			PyErr_SetString(PyExc_TypeError,
>> +					"Expected a "
>> +#if IS_PY3
>> +					"bytes "
>> +#else
>> +					"string "
>> +#endif
>> +					"argument to GUID()");
> I think I would prefer something like the attached patch, which
> concentrates the #ifdef-ing in one place.
that's much nicer, I thought about doing something like that but forgot
about doing it later.
I'll push a new version of the branch with the above (and hopefully
complete removal of the extra comments)

On 28/02/18 05:40, Andrew Bartlett wrote:
> [17(647)/2249 at 17m4s] samba.tests.dcerpc.misc.python3
> UNEXPECTED(error):
> samba.tests.dcerpc.misc.python3.samba.tests.dcerpc.misc.GUIDTests.test_
> compare_different(none)
> REASON: Exception: Exception: Traceback (most recent call last):
>   File
> "/home/ubuntu/autobuild/b32562/samba/bin/python/samba/tests/dcerpc/misc
> .py", line 45, in test_compare_different
>     guid1 = misc.GUID(text1)
> TypeError: Expected a bytes argument to GUID()
>
> FAILED (0 failures, 1 errors and 0 unexpected successes in 0
> testsuites)
>
> Sadly travis-ci never attempted this branch, so this wasn't picked up
I will add a modification for the test to make it pass. I don't
understand 'travis-ci never attempted this branch' afaics travis checks
ran for the branch or is it that the travis-ci doesn't perform all tests ?

Noel