[oslo] fix flake8-hacking inconsistences on xena/wallaby
Hello Osloers, As you surely recently observed patches proposed by the openstack bot to configure wallaby fail with a flake8/hacking issue. Here are some guideline to fix the problem the inconsistency: - Patch pre-commit on master (now xena) (here is the patch: https://gist.github.com/4383/6e96c836bb1b0e1e0e599a5106f43f1a) - Once Xena is patched cherry-pick and backport these changes on Wallaby - Rebase the openstack bot patches on the top of this cherry-pick (or wait for the merge of the previous one patch) You can copy the patch on oslo.db with the related commit message [1]. The root cause of the issue was that with the introduction of pre-commit we started to define the version of flake8 to use. Previously this version was defined by hacking's requirements. Indeed a few months ago we added pre-commit to allow us to run checks with git hooks and reduce the usage of our gates. These changes were standardized and spread on all the scope of oslo [2]. However, during the design of these changes [3] and after some discussion we decided to pin the version of flake8 to use, hence by doing this we short circuited hacking on its management of flake8. The solution to solve this issue is simply to trust hacking on its flake8 management. Hacking will pull the right version of flake8 and the inconsistency will disappear. flake8 provides a pre-commit hook so it could be seen and called as a local target. [1] https://review.opendev.org/c/openstack/oslo.db/+/781470 [2] https://review.opendev.org/q/topic:%22oslo-pre-commit%22 [3] https://review.opendev.org/q/topic:%22pre-commit%22+(status:open%20OR%20stat... -- Hervé Beraud Senior Software Engineer at Red Hat irc: hberaud https://github.com/4383/ https://twitter.com/4383hberaud -----BEGIN PGP SIGNATURE----- wsFcBAABCAAQBQJb4AwCCRAHwXRBNkGNegAALSkQAHrotwCiL3VMwDR0vcja10Q+ Kf31yCutl5bAlS7tOKpPQ9XN4oC0ZSThyNNFVrg8ail0SczHXsC4rOrsPblgGRN+ RQLoCm2eO1AkB0ubCYLaq0XqSaO+Uk81QxAPkyPCEGT6SRxXr2lhADK0T86kBnMP F8RvGolu3EFjlqCVgeOZaR51PqwUlEhZXZuuNKrWZXg/oRiY4811GmnvzmUhgK5G 5+f8mUg74hfjDbR2VhjTeaLKp0PhskjOIKY3vqHXofLuaqFDD+WrAy/NgDGvN22g glGfj472T3xyHnUzM8ILgAGSghfzZF5Skj2qEeci9cB6K3Hm3osj+PbvfsXE/7Kw m/xtm+FjnaywZEv54uCmVIzQsRIm1qJscu20Qw6Q0UiPpDFqD7O6tWSRKdX11UTZ hwVQTMh9AKQDBEh2W9nnFi9kzSSNu4OQ1dRMcYHWfd9BEkccezxHwUM4Xyov5Fe0 qnbfzTB1tYkjU78loMWFaLa00ftSxP/DtQ//iYVyfVNfcCwfDszXLOqlkvGmY1/Y F1ON0ONekDZkGJsDoS6QdiUSn8RZ2mHArGEWMV00EV5DCIbCXRvywXV43ckx8Z+3 B8qUJhBqJ8RS2F+vTs3DTaXqcktgJ4UkhYC2c1gImcPRyGrK9VY0sCT+1iA+wp/O v6rDpkeNksZ9fFSyoY2o =ECSj -----END PGP SIGNATURE-----
Here are some guideline to fix the problem the inconsistency: - Patch pre-commit on master (now xena) (here is the patch: https://gist.github.com/4383/6e96c836bb1b0e1e0e599a5106f43f1a <https://gist.github.com/4383/6e96c836bb1b0e1e0e599a5106f43f1a>) - Once Xena is patched cherry-pick and backport these changes on Wallaby - Rebase the openstack bot patches on the top of this cherry-pick (or wait for the merge of the previous one patch) I have made the first part at the moment. The topic is move_flake8_local for it. I need to clean several patches and I waiting
Le 23/03/2021 à 12:00, Herve Beraud a écrit : the merged before cherry-pick in wallaby. I saw also several patches failed because we have an issue in the lower-constraints file. Sometimes we have already a patch to remove this file, sometimes no. I will remove it from all repositories.
As one of the early adopters and supporters of pre-commit *tool* I should mention few things I observed. pre-commit tool by design does pin all the hooks/linters/checks, so it is mainly making use of 'hacking' package irrelevant. On many tripleo repositories we ended up removing hacking and I do not remember getting into any problems due to flake8 or its plugins so far.. My personal recommendation is to avoid the use of "repo: local", especial for calling an external tools (flake8). This makes you loose the ability to upgrade the linter using `pre-commit auto-update`. Local hooks are designed for running custom checks hosted in-repo. Sidenote: when using additional dependencies with pre-commit, there is no pinning, so there is still a chance you may want to use hacking or at least manually control the version of the extra packages you want to inject inside the "hook". If we still have a strong case for using hacking, I think it should be converted to be usable as a hook itself, one that calls flake8. If doing this it will be possible to make use of pre-commit pin to tag and fully control the bumping of flake8 with all the deps involved. -- /zbr On 23 Mar 2021 at 11:00:42, Herve Beraud <hberaud@redhat.com> wrote:
Hello Osloers,
As you surely recently observed patches proposed by the openstack bot to configure wallaby fail with a flake8/hacking issue.
Here are some guideline to fix the problem the inconsistency: - Patch pre-commit on master (now xena) (here is the patch: https://gist.github.com/4383/6e96c836bb1b0e1e0e599a5106f43f1a) - Once Xena is patched cherry-pick and backport these changes on Wallaby - Rebase the openstack bot patches on the top of this cherry-pick (or wait for the merge of the previous one patch)
You can copy the patch on oslo.db with the related commit message [1].
The root cause of the issue was that with the introduction of pre-commit we started to define the version of flake8 to use. Previously this version was defined by hacking's requirements.
Indeed a few months ago we added pre-commit to allow us to run checks with git hooks and reduce the usage of our gates. These changes were standardized and spread on all the scope of oslo [2].
However, during the design of these changes [3] and after some discussion we decided to pin the version of flake8 to use, hence by doing this we short circuited hacking on its management of flake8.
The solution to solve this issue is simply to trust hacking on its flake8 management. Hacking will pull the right version of flake8 and the inconsistency will disappear. flake8 provides a pre-commit hook so it could be seen and called as a local target.
[1] https://review.opendev.org/c/openstack/oslo.db/+/781470 [2] https://review.opendev.org/q/topic:%22oslo-pre-commit%22 [3] https://review.opendev.org/q/topic:%22pre-commit%22+(status:open%20OR%20stat...
-- Hervé Beraud Senior Software Engineer at Red Hat irc: hberaud https://github.com/4383/ https://twitter.com/4383hberaud -----BEGIN PGP SIGNATURE-----
wsFcBAABCAAQBQJb4AwCCRAHwXRBNkGNegAALSkQAHrotwCiL3VMwDR0vcja10Q+ Kf31yCutl5bAlS7tOKpPQ9XN4oC0ZSThyNNFVrg8ail0SczHXsC4rOrsPblgGRN+ RQLoCm2eO1AkB0ubCYLaq0XqSaO+Uk81QxAPkyPCEGT6SRxXr2lhADK0T86kBnMP F8RvGolu3EFjlqCVgeOZaR51PqwUlEhZXZuuNKrWZXg/oRiY4811GmnvzmUhgK5G 5+f8mUg74hfjDbR2VhjTeaLKp0PhskjOIKY3vqHXofLuaqFDD+WrAy/NgDGvN22g glGfj472T3xyHnUzM8ILgAGSghfzZF5Skj2qEeci9cB6K3Hm3osj+PbvfsXE/7Kw m/xtm+FjnaywZEv54uCmVIzQsRIm1qJscu20Qw6Q0UiPpDFqD7O6tWSRKdX11UTZ hwVQTMh9AKQDBEh2W9nnFi9kzSSNu4OQ1dRMcYHWfd9BEkccezxHwUM4Xyov5Fe0 qnbfzTB1tYkjU78loMWFaLa00ftSxP/DtQ//iYVyfVNfcCwfDszXLOqlkvGmY1/Y F1ON0ONekDZkGJsDoS6QdiUSn8RZ2mHArGEWMV00EV5DCIbCXRvywXV43ckx8Z+3 B8qUJhBqJ8RS2F+vTs3DTaXqcktgJ4UkhYC2c1gImcPRyGrK9VY0sCT+1iA+wp/O v6rDpkeNksZ9fFSyoY2o =ECSj -----END PGP SIGNATURE-----
Hello Sorin, Thanks for your feedback, they are much appreciated. Le mer. 24 mars 2021 à 13:45, Sorin Sbarnea <ssbarnea@redhat.com> a écrit :
As one of the early adopters and supporters of pre-commit *tool* I should mention few things I observed.
pre-commit tool by design does pin all the hooks/linters/checks, so it is mainly making use of 'hacking' package irrelevant. On many tripleo repositories we ended up removing hacking and I do not remember getting into any problems due to flake8 or its plugins so far..
Here our problem was that we recently started to define the version of flake8 to use and that version wasn't compatible with the one supported by hacking. The solution is simply to let hacking manage these own requirements. We don't want to drop hacking.
My personal recommendation is to avoid the use of "repo: local", especial for calling an external tools (flake8). This makes you loose the ability to upgrade the linter using `pre-commit auto-update`. Local hooks are designed for running custom checks hosted in-repo.
We don't want to use auto-update. We want to control the flow of supported hacking versions and its supported rules. We don't want to suffer each new version of hacking.
Sidenote: when using additional dependencies with pre-commit, there is no pinning, so there is still a chance you may want to use hacking or at least manually control the version of the extra packages you want to inject inside the "hook".
Please can you share docs or links that refer to this point? I didn't find that point in the official documentation. https://pre-commit.com/#config-additional_dependencies
If we still have a strong case for using hacking, I think it should be converted to be usable as a hook itself, one that calls flake8. If doing this it will be possible to make use of pre-commit pin to tag and fully control the bumping of flake8 with all the deps involved.
Indeed, it's a good idea. --
/zbr
On 23 Mar 2021 at 11:00:42, Herve Beraud <hberaud@redhat.com> wrote:
Hello Osloers,
As you surely recently observed patches proposed by the openstack bot to configure wallaby fail with a flake8/hacking issue.
Here are some guideline to fix the problem the inconsistency: - Patch pre-commit on master (now xena) (here is the patch: https://gist.github.com/4383/6e96c836bb1b0e1e0e599a5106f43f1a) - Once Xena is patched cherry-pick and backport these changes on Wallaby - Rebase the openstack bot patches on the top of this cherry-pick (or wait for the merge of the previous one patch)
You can copy the patch on oslo.db with the related commit message [1].
The root cause of the issue was that with the introduction of pre-commit we started to define the version of flake8 to use. Previously this version was defined by hacking's requirements.
Indeed a few months ago we added pre-commit to allow us to run checks with git hooks and reduce the usage of our gates. These changes were standardized and spread on all the scope of oslo [2].
However, during the design of these changes [3] and after some discussion we decided to pin the version of flake8 to use, hence by doing this we short circuited hacking on its management of flake8.
The solution to solve this issue is simply to trust hacking on its flake8 management. Hacking will pull the right version of flake8 and the inconsistency will disappear. flake8 provides a pre-commit hook so it could be seen and called as a local target.
[1] https://review.opendev.org/c/openstack/oslo.db/+/781470 [2] https://review.opendev.org/q/topic:%22oslo-pre-commit%22 [3] https://review.opendev.org/q/topic:%22pre-commit%22+(status:open%20OR%20stat...
-- Hervé Beraud Senior Software Engineer at Red Hat irc: hberaud https://github.com/4383/ https://twitter.com/4383hberaud -----BEGIN PGP SIGNATURE-----
wsFcBAABCAAQBQJb4AwCCRAHwXRBNkGNegAALSkQAHrotwCiL3VMwDR0vcja10Q+ Kf31yCutl5bAlS7tOKpPQ9XN4oC0ZSThyNNFVrg8ail0SczHXsC4rOrsPblgGRN+ RQLoCm2eO1AkB0ubCYLaq0XqSaO+Uk81QxAPkyPCEGT6SRxXr2lhADK0T86kBnMP F8RvGolu3EFjlqCVgeOZaR51PqwUlEhZXZuuNKrWZXg/oRiY4811GmnvzmUhgK5G 5+f8mUg74hfjDbR2VhjTeaLKp0PhskjOIKY3vqHXofLuaqFDD+WrAy/NgDGvN22g glGfj472T3xyHnUzM8ILgAGSghfzZF5Skj2qEeci9cB6K3Hm3osj+PbvfsXE/7Kw m/xtm+FjnaywZEv54uCmVIzQsRIm1qJscu20Qw6Q0UiPpDFqD7O6tWSRKdX11UTZ hwVQTMh9AKQDBEh2W9nnFi9kzSSNu4OQ1dRMcYHWfd9BEkccezxHwUM4Xyov5Fe0 qnbfzTB1tYkjU78loMWFaLa00ftSxP/DtQ//iYVyfVNfcCwfDszXLOqlkvGmY1/Y F1ON0ONekDZkGJsDoS6QdiUSn8RZ2mHArGEWMV00EV5DCIbCXRvywXV43ckx8Z+3 B8qUJhBqJ8RS2F+vTs3DTaXqcktgJ4UkhYC2c1gImcPRyGrK9VY0sCT+1iA+wp/O v6rDpkeNksZ9fFSyoY2o =ECSj -----END PGP SIGNATURE-----
-- Hervé Beraud Senior Software Engineer at Red Hat irc: hberaud https://github.com/4383/ https://twitter.com/4383hberaud -----BEGIN PGP SIGNATURE----- wsFcBAABCAAQBQJb4AwCCRAHwXRBNkGNegAALSkQAHrotwCiL3VMwDR0vcja10Q+ Kf31yCutl5bAlS7tOKpPQ9XN4oC0ZSThyNNFVrg8ail0SczHXsC4rOrsPblgGRN+ RQLoCm2eO1AkB0ubCYLaq0XqSaO+Uk81QxAPkyPCEGT6SRxXr2lhADK0T86kBnMP F8RvGolu3EFjlqCVgeOZaR51PqwUlEhZXZuuNKrWZXg/oRiY4811GmnvzmUhgK5G 5+f8mUg74hfjDbR2VhjTeaLKp0PhskjOIKY3vqHXofLuaqFDD+WrAy/NgDGvN22g glGfj472T3xyHnUzM8ILgAGSghfzZF5Skj2qEeci9cB6K3Hm3osj+PbvfsXE/7Kw m/xtm+FjnaywZEv54uCmVIzQsRIm1qJscu20Qw6Q0UiPpDFqD7O6tWSRKdX11UTZ hwVQTMh9AKQDBEh2W9nnFi9kzSSNu4OQ1dRMcYHWfd9BEkccezxHwUM4Xyov5Fe0 qnbfzTB1tYkjU78loMWFaLa00ftSxP/DtQ//iYVyfVNfcCwfDszXLOqlkvGmY1/Y F1ON0ONekDZkGJsDoS6QdiUSn8RZ2mHArGEWMV00EV5DCIbCXRvywXV43ckx8Z+3 B8qUJhBqJ8RS2F+vTs3DTaXqcktgJ4UkhYC2c1gImcPRyGrK9VY0sCT+1iA+wp/O v6rDpkeNksZ9fFSyoY2o =ECSj -----END PGP SIGNATURE-----
On 2021-03-24 05:44:59 -0700 (-0700), Sorin Sbarnea wrote: [...]
If we still have a strong case for using hacking, I think it should be converted to be usable as a hook itself, one that calls flake8. [...]
Can you expand on how you would expect that to work? Quoting from hacking's README: "hacking is a set of flake8 plugins that test and enforce the OpenStack StyleGuide" How would you turn a set of flake8 plugins into a pre-commit hook which calls flake8? That seems rather circular. -- Jeremy Stanley
Le mer. 24 mars 2021 à 17:13, Jeremy Stanley <fungi@yuggoth.org> a écrit :
On 2021-03-24 05:44:59 -0700 (-0700), Sorin Sbarnea wrote: [...]
If we still have a strong case for using hacking, I think it should be converted to be usable as a hook itself, one that calls flake8. [...]
Can you expand on how you would expect that to work? Quoting from hacking's README:
"hacking is a set of flake8 plugins that test and enforce the OpenStack StyleGuide"
How would you turn a set of flake8 plugins into a pre-commit hook which calls flake8? That seems rather circular.
Excellent point! --
Jeremy Stanley
-- Hervé Beraud Senior Software Engineer at Red Hat irc: hberaud https://github.com/4383/ https://twitter.com/4383hberaud -----BEGIN PGP SIGNATURE----- wsFcBAABCAAQBQJb4AwCCRAHwXRBNkGNegAALSkQAHrotwCiL3VMwDR0vcja10Q+ Kf31yCutl5bAlS7tOKpPQ9XN4oC0ZSThyNNFVrg8ail0SczHXsC4rOrsPblgGRN+ RQLoCm2eO1AkB0ubCYLaq0XqSaO+Uk81QxAPkyPCEGT6SRxXr2lhADK0T86kBnMP F8RvGolu3EFjlqCVgeOZaR51PqwUlEhZXZuuNKrWZXg/oRiY4811GmnvzmUhgK5G 5+f8mUg74hfjDbR2VhjTeaLKp0PhskjOIKY3vqHXofLuaqFDD+WrAy/NgDGvN22g glGfj472T3xyHnUzM8ILgAGSghfzZF5Skj2qEeci9cB6K3Hm3osj+PbvfsXE/7Kw m/xtm+FjnaywZEv54uCmVIzQsRIm1qJscu20Qw6Q0UiPpDFqD7O6tWSRKdX11UTZ hwVQTMh9AKQDBEh2W9nnFi9kzSSNu4OQ1dRMcYHWfd9BEkccezxHwUM4Xyov5Fe0 qnbfzTB1tYkjU78loMWFaLa00ftSxP/DtQ//iYVyfVNfcCwfDszXLOqlkvGmY1/Y F1ON0ONekDZkGJsDoS6QdiUSn8RZ2mHArGEWMV00EV5DCIbCXRvywXV43ckx8Z+3 B8qUJhBqJ8RS2F+vTs3DTaXqcktgJ4UkhYC2c1gImcPRyGrK9VY0sCT+1iA+wp/O v6rDpkeNksZ9fFSyoY2o =ECSj -----END PGP SIGNATURE-----
This is all is needed to make hacking being directly consumable by pre-commit (tool, not git hook): https://review.opendev.org/c/openstack/hacking/+/783820 I already did some tests and it worked fine, that is how I made bashate and doc8 long time ago: https://github.com/openstack/bashate/blob/master/.pre-commit-hooks.yaml https://github.com/PyCQA/doc8/blob/master/.pre-commit-hooks.yaml Once that is merged and released, you could replace the normal flake8 usage inside your repo .pre-commit-config.yaml file: - repo: https://review.opendev.org/openstack/hacking rev: 4.1.0 # or whatever release number we will pick hooks: - id: hacking If you want to add your own extra plugins you can still do it using the additional_dependencies key. -- /zbr On 24 Mar 2021 at 16:34:09, Herve Beraud <hberaud@redhat.com> wrote:
Le mer. 24 mars 2021 à 17:13, Jeremy Stanley <fungi@yuggoth.org> a écrit :
On 2021-03-24 05:44:59 -0700 (-0700), Sorin Sbarnea wrote: [...]
If we still have a strong case for using hacking, I think it should be converted to be usable as a hook itself, one that calls flake8. [...]
Can you expand on how you would expect that to work? Quoting from hacking's README:
"hacking is a set of flake8 plugins that test and enforce the OpenStack StyleGuide"
How would you turn a set of flake8 plugins into a pre-commit hook which calls flake8? That seems rather circular.
Excellent point!
--
Jeremy Stanley
-- Hervé Beraud Senior Software Engineer at Red Hat irc: hberaud https://github.com/4383/ https://twitter.com/4383hberaud -----BEGIN PGP SIGNATURE-----
wsFcBAABCAAQBQJb4AwCCRAHwXRBNkGNegAALSkQAHrotwCiL3VMwDR0vcja10Q+ Kf31yCutl5bAlS7tOKpPQ9XN4oC0ZSThyNNFVrg8ail0SczHXsC4rOrsPblgGRN+ RQLoCm2eO1AkB0ubCYLaq0XqSaO+Uk81QxAPkyPCEGT6SRxXr2lhADK0T86kBnMP F8RvGolu3EFjlqCVgeOZaR51PqwUlEhZXZuuNKrWZXg/oRiY4811GmnvzmUhgK5G 5+f8mUg74hfjDbR2VhjTeaLKp0PhskjOIKY3vqHXofLuaqFDD+WrAy/NgDGvN22g glGfj472T3xyHnUzM8ILgAGSghfzZF5Skj2qEeci9cB6K3Hm3osj+PbvfsXE/7Kw m/xtm+FjnaywZEv54uCmVIzQsRIm1qJscu20Qw6Q0UiPpDFqD7O6tWSRKdX11UTZ hwVQTMh9AKQDBEh2W9nnFi9kzSSNu4OQ1dRMcYHWfd9BEkccezxHwUM4Xyov5Fe0 qnbfzTB1tYkjU78loMWFaLa00ftSxP/DtQ//iYVyfVNfcCwfDszXLOqlkvGmY1/Y F1ON0ONekDZkGJsDoS6QdiUSn8RZ2mHArGEWMV00EV5DCIbCXRvywXV43ckx8Z+3 B8qUJhBqJ8RS2F+vTs3DTaXqcktgJ4UkhYC2c1gImcPRyGrK9VY0sCT+1iA+wp/O v6rDpkeNksZ9fFSyoY2o =ECSj -----END PGP SIGNATURE-----
On 2021-03-30 05:42:25 -0400 (-0400), Sorin Sbarnea wrote:
This is all is needed to make hacking being directly consumable by pre-commit (tool, not git hook): https://review.opendev.org/c/openstack/hacking/+/783820
I already did some tests and it worked fine, that is how I made bashate and doc8 long time ago:
https://github.com/openstack/bashate/blob/master/.pre-commit-hooks.yaml https://github.com/PyCQA/doc8/blob/master/.pre-commit-hooks.yaml
Once that is merged and released, you could replace the normal flake8 usage inside your repo .pre-commit-config.yaml file:
- repo: https://review.opendev.org/openstack/hacking rev: 4.1.0 # or whatever release number we will pick hooks: - id: hacking
If you want to add your own extra plugins you can still do it using the additional_dependencies key. [...]
I'm confused as to why you'd call it hacking when you're invoking flake8 with multiple plugins only one of which is hacking. It's just flake8 and the plugins you've installed for it, right? -- Jeremy Stanley
participants (4)
-
Daniel Bengtsson
-
Herve Beraud
-
Jeremy Stanley
-
Sorin Sbarnea