On 11/13/23 15:15, Ilya Maximets wrote:
> On 11/13/23 14:47, Dumitru Ceara wrote:
>> On 11/13/23 14:41, Ilya Maximets wrote:
>>> On 11/13/23 14:33, Dumitru Ceara wrote:
>>>> Without this, when using Python 3.12 and flake8 5.0.4, the following
>>>> errors are flagged:
>>>>   tests/check_acl_log.py:97:25: E231 missing whitespace after ':'
>>>>   tests/check_acl_log.py:102:71: E231 missing whitespace after ':'
>>>>
>>>> This was reported and discussed in a couple of contexts:
>>>> https://mail.openvswitch.org/pipermail/ovs-dev/2023-November/409325.html
>>>> https://mail.openvswitch.org/pipermail/ovs-dev/2023-November/409277.html
>>>>
>>>> And it's fixed in recent flake8/pycodestyle versions:
>>>> https://github.com/PyCQA/flake8/issues/1845#issuecomment-1766073353
>>>>
>>>> Unfortunately we have to remove the 'hacking' requirement because that
>>>> introduces a dependency on 'flake8<4.0.0 and >=3.6.0'.  That should be
>>>> OK though because 'hacking' is an OpenStack specific package and OVN
>>>> doesn't expose any Python code.
>>>>
>>>> Signed-off-by: Dumitru Ceara <[email protected]>
>>>> ---
>>>> NOTE: this patch should be backported to all supported branches.
>>>> ---
>>>>  utilities/containers/py-requirements.txt | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/utilities/containers/py-requirements.txt 
>>>> b/utilities/containers/py-requirements.txt
>>>> index 0d90765c97..a8e8f17da3 100644
>>>> --- a/utilities/containers/py-requirements.txt
>>>> +++ b/utilities/containers/py-requirements.txt
>>>> @@ -1,7 +1,7 @@
>>>> -flake8
>>>> -hacking>=3.0
>>>> +flake8>=6.1.0
>>>>  scapy
>>>>  sphinx
>>>>  setuptools
>>>>  pyelftools
>>>>  pyOpenSSL
>>>> +pycodestyle>=2.11.0
>>>
>>>
>>> Alternative might be to still install hacking for python <3.12,
>>> but do not otherwise.
>>>
>>> See https://peps.python.org/pep-0508/#environment-markers
>>>
>>
>> I'm not sure what benefit we get from 'hacking' to be honest.  OVN uses
>> python only in ovn-detrace (the rest of the python code is test code)
>> and that is not exported as a package to anyone else to use.
>>
>> Personally, I'd just remove it (hence the patch :D) but I'll wait for
>> more input on this.  In the meantime our CI is broken for 23.06.
> 
> The problem with that is that you're only removing hacking from CI,
> but not from the documentation or a build process.  So, CI will no
> longer cover it, but users may have build failures locally.
> 

Ack, you're right, I should be updating the documentation and build too.

For the latter, all the checks we currently enable are:
#   H231 Python 3.x incompatible 'except x,y:' construct
#   H232 Python 3.x incompatible octal 077 should be written as 0o77
#   H233 Python 3.x incompatible use of print operator
#   H238 old style class declaration, use new style (inherit from `object`)
FLAKE8_SELECT = H231,H232,H233,H238

These are skipped if the interpreter is Python 2.  All other hacking
checks are ignored (it's part of FLAKE8_IGNORE).

We explicitly require Python 3 support and we don't support Python 2
anymore:

https://github.com/ovn-org/ovn/commit/0c042c2d28d87b90300ca86fe67427dab908134c

It's safe to remove all hacking checks from the build.  I'll do that in
v2.  While we're at it I guess we should do the same thing for OVS.
What do you think?

Thanks,
Dumitru

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to