On 11/14/23 11:58, Ilya Maximets wrote: > On 11/14/23 11:43, Dumitru Ceara wrote: >> On 11/14/23 11:33, Ilya Maximets wrote: >>> On 11/14/23 11:26, Ilya Maximets wrote: >>>> On 11/13/23 17:45, 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'. On the other >>>>> hand the currently enabled hacking checks were only applicable to >>>>> Python 2 code. OVN has stopped supporting Python 2 for a while now, >>>>> since 0c042c2d28d8 ("Require Python 3 and remove support for Python >>>>> 2."). >>>>> >>>>> Signed-off-by: Dumitru Ceara <dce...@redhat.com> >>>>> --- >>>>> NOTE: this patch should be backported to all supported branches. >>>>> >>>>> V2: >>>>> - Addressed Ilya's comments: >>>>> - removed remaining hacking references >>>>> - updated commit log >>>>> --- >>>>> Documentation/intro/install/general.rst | 5 +---- >>>>> Makefile.am | 9 +-------- >>>>> utilities/containers/py-requirements.txt | 4 ++-- >>>>> 3 files changed, 4 insertions(+), 14 deletions(-) >>>>> >>>>> diff --git a/Documentation/intro/install/general.rst >>>>> b/Documentation/intro/install/general.rst >>>>> index 5895188462..4ace64f6ec 100644 >>>>> --- a/Documentation/intro/install/general.rst >>>>> +++ b/Documentation/intro/install/general.rst >>>>> @@ -134,10 +134,7 @@ following to obtain better warnings: >>>>> >>>>> - clang, version 3.4 or later >>>>> >>>>> -- flake8 along with the hacking flake8 plugin (for Python code). The >>>>> automatic >>>>> - flake8 check that runs against Python code has some warnings enabled >>>>> that >>>>> - come from the "hacking" flake8 plugin. If it's not installed, the >>>>> warnings >>>>> - just won't occur until it's run on a system with "hacking" installed. >>>>> +- flake8, version 6.1.0 or later >>>>> >>>>> You may find the ovs-dev script found in ``ovs/utilities/ovs-dev.py`` >>>>> useful. >>>>> >>>>> diff --git a/Makefile.am b/Makefile.am >>>>> index 06045760a0..d24cca10fb 100644 >>>>> --- a/Makefile.am >>>>> +++ b/Makefile.am >>>>> @@ -414,17 +414,10 @@ ALL_LOCAL += flake8-check >>>>> # F*** -- warnings native to flake8 >>>>> # F811 redefinition of unused <name> from line <N> (only from flake8 >>>>> v2.0) >>>>> # D*** -- warnings from flake8-docstrings plugin >>>>> -# H*** -- warnings from flake8 hacking plugin (custom style checks >>>>> beyond PEP8) >>> >>> We may also keep this line, since we're going to explicitly ignore these. >>> >>>>> -# 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 >>>>> -FLAKE8_IGNORE = >>>>> E121,E123,E125,E126,E127,E128,E129,E131,E722,W503,W504,F811,D,H,I >>>>> +FLAKE8_IGNORE = >>>>> E121,E123,E125,E126,E127,E128,E129,E131,E722,W503,W504,F811,D,I >>>> >>>> We need to keep H in the ignore list, because flake8 automatically enables >>>> them if the plugin is installed. That is causing build failures in CI and >>>> will cause build failures for everyone who has hacking installed. >>>> >> >> Oh, d'oh, I should've done more than testing locally. We get the >> pre-installed ubuntu image in CI from ghcr.io/ovn-org/ovn-tests:ubuntu >> and that gets rebuilt once a week. The one that got pulled had hacking >> installed. > > This is kind of a problem though. If we will ever need to change > some dependencies in order to fix CI failure we'll have to merge > the fix without CI and trigger the container rebuild manually. > That doesn't sound great. >
The workflow can indeed be triggered manually too; I'm not sure if we can do better. OTOH this should be the exception, most patches won't change CI. > IIUC, the current state of GHA integration also doesn't allow > testing with custom dependencies in a separate repo during some > feature development. > You can specify the image to use and make it point to your own fork via the IMAGE_NAME env variable in the test.yml workflow file. You do need a custom commit for that though. >> >> I'll leave the "hacking" ignore and the comment above and post v3. >> >> Thanks for pointing it out! >> >>>> Best regards, Ilya Maximets. >>>> >>>>> flake8-check: $(FLAKE8_PYFILES) >>>>> $(FLAKE8_WERROR)$(AM_V_GEN) \ >>>>> src='$^' && \ >>>>> - flake8 $$src --select=$(FLAKE8_SELECT) $(FLAKE8_FLAGS) && \ >>>>> flake8 $$src --ignore=$(FLAKE8_IGNORE) $(FLAKE8_FLAGS) && \ >>>>> touch $@ >>>>> endif >>>>> 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 >>>> >>> >> > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
