| Message ID | 20231113164509.813553-1-dceara@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v2] py-requirements: Remove hacking dependency and use recent flake8. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
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 <dceara@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) > -# 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. 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
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 <dceara@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. > > 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 >
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 <dceara@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. 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 >> >
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 <dceara@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. IIUC, the current state of GHA integration also doesn't allow testing with custom dependencies in a separate repo during some feature development. > > 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 >>> >> >
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 <dceara@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 >>>> >>> >> >
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) -# 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 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
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 <dceara@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(-)