Bug 220510

Summary: [ews] Add python 3 support - part 1
Product: WebKit Reporter: Aakash Jain <aakash_jain>
Component: Tools / TestsAssignee: Aakash Jain <aakash_jain>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, jbedard, ryanhaddad, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=221899
https://bugs.webkit.org/show_bug.cgi?id=222355
https://bugs.webkit.org/show_bug.cgi?id=222672
Attachments:
Description Flags
Patch none

Description Aakash Jain 2021-01-11 06:59:55 PST
Add Python 3 support to ews code.
Comment 1 Aakash Jain 2021-01-11 07:09:52 PST
Created attachment 417377 [details]
Patch
Comment 2 EWS 2021-01-11 08:44:35 PST
Committed r271361: <https://trac.webkit.org/changeset/271361>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 417377 [details].
Comment 3 Radar WebKit Bug Importer 2021-01-11 08:45:21 PST
<rdar://problem/72999194>
Comment 4 Alexey Proskuryakov 2021-01-11 16:55:54 PST
Comment on attachment 417377 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=417377&action=review

> Tools/ChangeLog:3
> +        [ews] Add python 3 support - part 1

This looks like moving to python 3 while dropping python 2, was this the intention?

> Tools/CISupport/ews-build/email_unittest.py:41
> -            self.assertTrue(category in emails.keys())
> +            self.assertTrue(category in list(emails.keys()))

Is this actually necessary? I think that converting to a list just makes the code slower.

> Tools/CISupport/ews-build/loadConfig_unittest.py:95
> +                    self.assertTrue(value[0] in list(schedulers_to_buildername_map.keys()),

Ditto.
Comment 5 Aakash Jain 2021-01-11 18:23:23 PST
Comment on attachment 417377 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=417377&action=review

>> Tools/ChangeLog:3
>> +        [ews] Add python 3 support - part 1
> 
> This looks like moving to python 3 while dropping python 2, was this the intention?

The intention was to make the code compatible with both python 2 and 3.

>> Tools/CISupport/ews-build/email_unittest.py:41
>> +            self.assertTrue(category in list(emails.keys()))
> 
> Is this actually necessary? I think that converting to a list just makes the code slower.

Not really. It makes the 2to3 tool happy though.
This is a very small dictionary, and that too in unit-tests, the performance difference wouldn't really be noticeable in this case.
Comment 6 Jonathan Bedard 2021-02-15 12:07:26 PST
Comment on attachment 417377 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=417377&action=review

>>> Tools/CISupport/ews-build/email_unittest.py:41
>>> +            self.assertTrue(category in list(emails.keys()))
>> 
>> Is this actually necessary? I think that converting to a list just makes the code slower.
> 
> Not really. It makes the 2to3 tool happy though.
> This is a very small dictionary, and that too in unit-tests, the performance difference wouldn't really be noticeable in this case.

No reason to do this here. The reason the 2to3 tool blindly does this is because in most operations, the difference matters. The exception is iteration and the 'in' operator.