[PATCH] Python 3 porting for selftest.ndrdump

joeg at catalyst.net.nz joeg at catalyst.net.nz
Fri Mar 23 02:47:14 UTC 2018 

Hi, everyone,

About this pull request:

https://github.com/samba-team/samba/pull/146

After fixing the var name typo, it passes the autobuild test.
I make a new patch with the fix.
Can any one review it then we can push this into master, please?

On 22/03/18 23:20, Noel Power via samba-technical wrote:
> <sigh> I keep forgetting that the github ci tests are not the same as
> autobuild. I've never run autobuild (I always thought is was something
> running on some samba-team server somewhere that samba team members push
> to) I have to either figure out what you need to do or run all tests
> manually for some of these changes I guess
>
> Myself and David will fix these errors and repush
>
> Noel
> On 22/03/18 02:22, joeg at catalyst.net.nz wrote:
>> Hi Noel,
>>
>> I've applied these patches to latest github master:
>>
>> https://github.com/samba-team/samba/pull/125.patch
>> https://github.com/samba-team/samba/pull/126.patch
>> https://github.com/samba-team/samba/pull/127.patch
>> https://github.com/samba-team/samba/pull/146.patch
>> https://github.com/samba-team/samba/pull/147.patch
>>
>> And triggered autobuild, I got these errors:
>>
>> ==> samba-none-env.stdout <==
>> [1(0)/210 at 0s] samba.tests.source
>> UNEXPECTED(failure):
>> samba.tests.source.samba.tests.source.TestSource.test_copyright(none)
>> REASON: Exception: Exception: Traceback (most recent call last):
>>   File
>> "/tmp/samba-testbase/b18/samba-none-env/bin/python/samba/tests/source.py",
>> line 104, in test_copyright
>>     self.fail('\n'.join(help_text))
>> AssertionError: Some files have missing or incorrect copyright statements.
>>
>> /tmp/samba-testbase/b18/samba-none-env/bin/python/samba/tests/dckeytab.py
>>     no copyright line found
>>
>> /tmp/samba-testbase/b18/samba-none-env/bin/python/samba/tests/smb.py
>>     no copyright line found
>>
>> /tmp/samba-testbase/b18/samba-none-env/bin/python/samba/tests/gpo.py
>>     no copyright line found
>>
>> The above 3 files miss copyright line, minor.  And another minor one:
>>
>> [802(5024)/830 at 2h14m24s] samba4.drs.getncchanges.python(vampire_dc)(vampire_dc)
>> UNEXPECTED(error): samba4.drs.getncchanges.python(vampire_dc).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_anc_link_attr(vampire_dc)
>> REASON: Exception: Exception: Traceback (most recent call last):
>>   File "/tmp/samba-testbase/b14/samba/source4/torture/drs/python/getncchanges.py", line 663, in test_repl_get_anc_link_attr
>>     self.repl_get_next()
>>   File "/tmp/samba-testbase/b14/samba/source4/torture/drs/python/getncchanges.py", line 300, in repl_get_next
>>     print("Unknown parent for %s - try GET_ANC" % d)
>> NameError: global name 'd' is not defined
>>
>> The d in print should be dn.
>>
>> On 22/03/18 11:00, joeg--- via samba-technical wrote:
>>> Hi Noel,
>>>
>>> Thank you for the great input, it's very helpful to me.
>>>
>>> I agree with your plan, we need to port the c-modules first, and then
>>> python.
>>>
>>> I will get all the pending python3 porting related patches together, and
>>> trigger autobuild with gitlab ci.
>>>
>>> If the result is good, then we are more confident to get the code in,
>>> and move on to next issue.
>>>
>>> Let's see what will happen.
>>>
>>>
>>> On 22/03/18 06:14, Noel Power via samba-technical wrote:
>>>> Hi Joe,
>>>>
>>>> First of all great to see another body working on this. However I really
>>>> fear that unless we are careful there is a danger of duplication of
>>>> effort here with some work that has already been done by Lumir, myself &
>>>> David. Actually the patch you are working on falls into this category
>>>> (in the patch set mentioned in point 2 below)
>>>>
>>>> Regarding your plan below this is very similar to what we have tried. At
>>>> suse myself and David found quite a number of c-modules not converted to
>>>> python3. Lots of tests import modules which import other modules etc.
>>>> etc. as a result there can be quite a complicated mess of
>>>> interdependencies. Of course there are probably quite a fair number of
>>>> tests that can be enabled that don't require these missing 'c-modules'
>>>> and perhaps our experience was coloured by the tests that we tried to
>>>> enable. However it seemed to be successful and avoid any potential
>>>> import problems a good base to start from would be the availability of
>>>> all python3 modules so our plan is to
>>>>
>>>>  1. Port all the remaining c-modules that are used to need be converted
>>>> to python3. We think we have all of those converted. We have some of
>>>> those c-modules out for review.
>>>>
>>>>     https://github.com/samba-team/samba/pull/147
>>>>     https://github.com/samba-team/samba/pull/125
>>>>     https://github.com/samba-team/samba/pull/126
>>>>     https://github.com/samba-team/samba/pull/127
>>>>
>>>> Unfortunately there has been little progress with the above and
>>>> currently they are all afaics stalled.
>>>> Additionally we have quite a few more modules ported (but not for review
>>>> yet) some we are trying to write tests for,  and for others the existing
>>>> tests depend on patches like the above and are also stalled because of that.
>>>>
>>>>  2. There is a range of  python2 -> python3 known porting issues (such
>>>> as exception declarations,  number format, calls to print methods etc.)
>>>> Where we have experienced some of these and where it seemed like a bulk
>>>> change could be possibly with little risk we have tried to that. There
>>>> is still an outstanding review for the last one submitted
>>>>
>>>>    https://github.com/samba-team/samba/pull/146
>>>>
>>>>    There probably will be more or these (things like iteration behaviour
>>>> come to mind) these might be more complicated to change en-masse.
>>>>
>>>> 3. Start enabling piecemeal tests (of course we currently are focussing
>>>> on tests that are impacted/test the modules we are porting) The
>>>> intention though is to extend this once complete.
>>>>
>>>>
>>>> I am not sure what is the best way to proceed is to avoid duplication
>>>> and stepping over each others toes. Personally I don't think it makes
>>>> sense to block everybody else's progress with some long living branch
>>>> the everybody depends on. I would prefer if we could commit early and often.
>>>>
>>>> From my point of view the best way to get this working is to get the
>>>> outstanding reviews (and the queued up c-modules not currently out for
>>>> review) into master as soon as possible. I do believe that getting the
>>>> code in sooner rather than later is the best way forward as long as we
>>>> are as sure as we can that it doesn't impact the current python2
>>>>
>>>> The problem with the python3 port is not trivial, there needs to be a
>>>> critical mass of changes in place in order to be able to start porting
>>>> existing tests effectively. Not being domain experts with the many many
>>>> areas the code touches we will have to rely on getting tests enabled to
>>>> actually see potential problems with the api. The sooner the code is in
>>>> the sooner we will see those problems. Hopefully the changes we make
>>>> will only affect python3 behaviour so we have a window of opportunity to
>>>> shake down python3 whilst maintaining python2 working as normal.
>>>>
>>>> Noel
>>>>
>>>>
>>>> On 21/03/18 01:41, joeg at catalyst.net.nz wrote:
>>>>> Hi everyone:
>>>>>
>>>>> I am going to focus on the Python 3 porting task for 2 weeks or so.
>>>>>
>>>>> I was a Python Web developer before.  In this context, most of my
>>>>> knowledge are about Python,
>>>>>
>>>>> and I don't know too much about Samba internal yet.
>>>>>
>>>>> To make this Python 3 porting task measurable and operational to me,
>>>>> after a discussion with
>>>>>
>>>>> Andrew Bartlett, I decided to carry on it via selftest:
>>>>>
>>>>>
>>>>> 0.  Configure samba with `--extra-python /path/to/python3`
>>>>>
>>>>> 1.  Find a test suite in selftest/tests.py,  add `py3_compatible=True`.
>>>>>
>>>>> 2. Run above test suite, and fix errors.
>>>>>
>>>>> 3. Send patch to email list.
>>>>>
>>>>> 4. Move on to next one.
>>>>>
>>>>>
>>>>> This is the first patch, for `samba.tests.blackbox.ndrdump`, and it's
>>>>> simple. 
>>>>>
>>>>> In the meanwhile, I will work on next ones and send follow-up patches.
>>>>>
>>>>> Let's see whether this strategy works well.
>>>>>
>>>>> Review appreciated. Thanks!
>>>>>
>>>>>
>>>>> Joe Guo
>>>>>
>>>>> -- 
>>>>> Joe Guo
>>>>> joeg at catalyst.net.nz
>>>>> Catalyst IT
>> -- 
>> Joe Guo
>> joeg at catalyst.net.nz
>> Catalyst IT
>

-- 
Joe Guo
joeg at catalyst.net.nz
Catalyst IT

-------------- next part --------------
A non-text attachment was scrubbed...
Name: github-pr-146-bulk-python3-print-port.patch
Type: text/x-patch
Size: 199090 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180323/364e1f26/github-pr-146-bulk-python3-print-port-0001.bin>

More information about the samba-technical mailing list