1388019 - [mozlog] Add support for Python 3

Status: RESOLVED FIXED

(Testing :: Mozbase, enhancement)

Description

[Ed Morley [:emorley]]

Without dropping support for legacy Python, we need to add support for Python 3 to mozlog. Since mozlog depends on mozfile, we likely need to fix bug 1388018 first too.

Amongst other things, this will help Treeherder switch to Python 3 (mozlog is the only mozbase package it uses).

Comment 1

[Vedant Chakravadhanula(:vedantc98)]

Attachment: Bug 1388019 - Adding Python 3 support for mozlog.

Changed module imports SocketServer and StringIO
to use the module six.

Review commit: https://reviewboard.mozilla.org/r/207480/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/207480/

Comment 2

[Vedant Chakravadhanula(:vedantc98)]

Hey, I've added a patch with a few initial changes that are required to add py3 support.
Please let me know what else is required.

I'm unable to use python-test to test the module because it gives me an error as follows :
OSError: [Errno 62] Too many levels of symbolic links: '/Users/vedantc98/src/firefox/obj-x86_64-apple-darwin16.7.0/_virtualenv/bin/python'

I'm running macOS.
Thanks.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Please see bug 1416924 which might help you to get the error fixed.

Comment 4

[Vedant Chakravadhanula(:vedantc98)]

Comment on attachment 8936759 [details]
Bug 1388019 - Adding Python 3 support for mozlog.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/207480/diff/1-2/

Comment 5

[Vedant Chakravadhanula(:vedantc98)]

I've fixed the problem with python-test, and so I've fixed the error.
Further, I've run the code through a py2 and py3 linter using `mach lint -l py2 -l py3 testing/mozbase/mozlog/ `.

Since the updated review request returns no errors on the linter as well as python-test, mozlog should be ready for python 3 after Bug 1388018 is resolved.

Comment 6

[Ed Morley [:emorley]]

Comment on attachment 8936759 [details]
Bug 1388019 - Adding Python 3 support for mozlog.

Hi! Thank you for the patch. I trust Henrik to review this more than me :-)

Comment 7

[Vedant Chakravadhanula(:vedantc98)]

Comment on attachment 8936759 [details]
Bug 1388019 - Adding Python 3 support for mozlog.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/207480/diff/2-3/

Comment 8

[Vedant Chakravadhanula(:vedantc98)]

I'm sorry about the review flag, please could you set it back to Henrik? This patch was pushed by accident..

Comment 9

[Ed Morley [:emorley]]

Comment on attachment 8936759 [details]
Bug 1388019 - Adding Python 3 support for mozlog.

No problem - restoring now :-)

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

I'm barely around until early next year and William helped you already with other patches. So he is better suited to review this one. Thanks for working on it Verdant!

Comment 11

[William Lachance (:wlach)]

Comment on attachment 8936759 [details]
Bug 1388019 - Adding Python 3 support for mozlog.

https://reviewboard.mozilla.org/r/207480/#review214478

::: testing/mozbase/mozlog/mozlog/unstructured/loglistener.py
(Diff revision 3)
>  # You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  from __future__ import absolute_import
>  
> -import SocketServer
> +from six.moves import socketserver
> -import socket

I think we still need 'socket' here, since we still catch an exception from it below.

::: testing/mozbase/mozlog/setup.py:27
(Diff revision 3)
>        author_email='[e-mail]',
>        url='https://wiki.mozilla.org/Auto-tools/Projects/Mozbase',
>        license='MPL 1.1/GPL 2.0/LGPL 2.1',
>        packages=find_packages(),
>        zip_safe=False,
> -      install_requires=["blessings>=1.3"],
> +      install_requires=deps,

I don't see a need to break deps into a variable here, I think it's more clear just to define them inline.

::: testing/mozbase/mozlog/setup.py
(Diff revision 3)
>                     'Intended Audience :: Developers',
>                     'License :: OSI Approved :: Mozilla Public License 1.1 (MPL 1.1)',
>                     'Operating System :: OS Independent',
> -                   'Topic :: Software Development :: Libraries :: Python Modules',
> +                   'Topic :: Software Development :: Libraries :: Python Modules'],
> -                   'Programming Language :: Python :: 2.7',
> -                   'Programming Language :: Python :: 2 :: Only'],

Could you set the version classifiers as we did for mozprofile and mozfile?

Comment 12

[Vedant Chakravadhanula(:vedantc98)]

Comment on attachment 8936759 [details]
Bug 1388019 - Adding Python 3 support for mozlog.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/207480/diff/3-4/

Comment 13

[Vedant Chakravadhanula(:vedantc98)]

Added the changes, plus a couple that i had missed earlier.
Thanks!

Comment 14

[Ed Morley [:emorley]]

Comment on attachment 8936759 [details]
Bug 1388019 - Adding Python 3 support for mozlog.

Reassigning since Will did the previous review round :-)

Comment 15

[William Lachance (:wlach)]

Comment on attachment 8936759 [details]
Bug 1388019 - Adding Python 3 support for mozlog.

https://reviewboard.mozilla.org/r/207480/#review215138

Looks pretty close here too! I'll trigger another try run.

::: testing/mozbase/mozlog/setup.py:25
(Diff revision 4)
>        packages=find_packages(),
>        zip_safe=False,
> -      install_requires=["blessings>=1.3"],
> -      tests_require=['mozfile'],
> +      install_requires=['blessings >= 1.3',
> +                        'six >= 1.10.0'],
> +      tests_require=['mozfile',
> +                     'six>=1.10.0'

Since you specified six in install_requires, you shouldn't need to specify it again in tests_require.

Comment 16

[Vedant Chakravadhanula(:vedantc98)]

Comment on attachment 8936759 [details]
Bug 1388019 - Adding Python 3 support for mozlog.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/207480/diff/4-5/

Comment 17

[Vedant Chakravadhanula(:vedantc98)]

I've made the changes and pushed them.

Comment 18

[William Lachance (:wlach)]

Comment on attachment 8936759 [details]
Bug 1388019 - Adding Python 3 support for mozlog.

https://reviewboard.mozilla.org/r/207480/#review215542

looks good now, thank you!

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ba4c8c662479
Adding Python 3 support for mozlog. r=wlach

Comment 20

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/ba4c8c662479