| Message ID | 20231114113852.886473-1-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v3] 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 | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Tue, Nov 14, 2023 at 12:38 PM Dumitru Ceara <dceara@redhat.com> 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. > > V3: > - Addressed Ilya's comments: > - keep hacking checks on flake8 ignore list for the case when hacking > is installed. > > V2: > - Addressed Ilya's comments: > - removed remaining hacking references > - updated commit log > --- > Documentation/intro/install/general.rst | 5 +---- > Makefile.am | 6 ------ > utilities/containers/py-requirements.txt | 4 ++-- > 3 files changed, 3 insertions(+), 12 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..b5c163f95e 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -415,16 +415,10 @@ ALL_LOCAL += flake8-check > # 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-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 > -- > 2.39.3 > > Looks good to me, thanks. Acked-by: Ales Musil <amusil@redhat.com>
On 11/14/23 12:38, 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. > > V3: > - Addressed Ilya's comments: > - keep hacking checks on flake8 ignore list for the case when hacking > is installed. > > V2: > - Addressed Ilya's comments: > - removed remaining hacking references > - updated commit log > --- > Documentation/intro/install/general.rst | 5 +---- > Makefile.am | 6 ------ > utilities/containers/py-requirements.txt | 4 ++-- > 3 files changed, 3 insertions(+), 12 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 Looks fine in general, but I'm not sure if we need to call out the version of flake8 specifically here. It's not really a dependency for OVN itself. It's just an incompatibility between older flake8 and the python 3.12 that has nothing to do with OVN. Maybe go back to a version before commit cd8747c54254 ("INSTALL.md: Note use of "hacking" flake8 plugin.") ? i.e.: - flake8 (for Python code) Either way: Acked-by: Ilya Maximets <i.maximets@ovn.org>
Thanks Dumitru, Acked-by: Mark Michelson <mmichels@redhat.com> On 11/14/23 06:38, 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. > > V3: > - Addressed Ilya's comments: > - keep hacking checks on flake8 ignore list for the case when hacking > is installed. > > V2: > - Addressed Ilya's comments: > - removed remaining hacking references > - updated commit log > --- > Documentation/intro/install/general.rst | 5 +---- > Makefile.am | 6 ------ > utilities/containers/py-requirements.txt | 4 ++-- > 3 files changed, 3 insertions(+), 12 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..b5c163f95e 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -415,16 +415,10 @@ ALL_LOCAL += flake8-check > # 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-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 15:57, Ilya Maximets wrote: > On 11/14/23 12:38, 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. >> >> V3: >> - Addressed Ilya's comments: >> - keep hacking checks on flake8 ignore list for the case when hacking >> is installed. >> >> V2: >> - Addressed Ilya's comments: >> - removed remaining hacking references >> - updated commit log >> --- >> Documentation/intro/install/general.rst | 5 +---- >> Makefile.am | 6 ------ >> utilities/containers/py-requirements.txt | 4 ++-- >> 3 files changed, 3 insertions(+), 12 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 > > Looks fine in general, but I'm not sure if we need to call out the > version of flake8 specifically here. It's not really a dependency > for OVN itself. It's just an incompatibility between older flake8 > and the python 3.12 that has nothing to do with OVN. > > Maybe go back to a version before commit cd8747c54254 ("INSTALL.md: > Note use of "hacking" flake8 plugin.") ? i.e.: > > - flake8 (for Python code) Sure, I changed it like that. > > Either way: > > Acked-by: Ilya Maximets <i.maximets@ovn.org> > Thanks, Ales, Ilya and Mark for the reviews! I pushed this to main and backported it to all stable branches down to 22.03. I did restrict flake8 to version flake8==5.0.4 on stable branches so that we can use tagged ovs releases as submodule version. CI is still unhappy but for a few other reasons: a. ARM CI needs an updated container (without hacking installed): I triggered a run manually now b. stable branches CI, <= 23.06, need to run with python 3.11 so we need custom container images for them; I'm trying to address that with the other series (needs v2): https://patchwork.ozlabs.org/project/ovn/cover/170004942125.996204.3468999446823223471.stgit@dceara.remote.csb/ Regards, Dumitru
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..b5c163f95e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -415,16 +415,10 @@ ALL_LOCAL += flake8-check # 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-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. V3: - Addressed Ilya's comments: - keep hacking checks on flake8 ignore list for the case when hacking is installed. V2: - Addressed Ilya's comments: - removed remaining hacking references - updated commit log --- Documentation/intro/install/general.rst | 5 +---- Makefile.am | 6 ------ utilities/containers/py-requirements.txt | 4 ++-- 3 files changed, 3 insertions(+), 12 deletions(-)