On 11/13/23 17:23, Dumitru Ceara wrote:
> 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?

Some of the checks are also not selected without --enable-extentions
in the latest versions of flake8, and we do not supply it.

Yeah, we should probably just remove the hacking from the OVS build
as well, since all these are just python 2-3 compatibility checks.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to