Re: [Openstack] PEP8 checks
On Mon, Jul 9, 2012 at 9:16 PM, Dan Prince wrote: > > > - Original Message - > > From: "Dave Walker" > > To: "Monty Taylor" > > Cc: openstack@lists.launchpad.net, "John Garbutt" < > john.garb...@citrix.com> > > Sent: Monday, July 9, 2012 6:01:19 PM > > Subject: Re: [Openstack] PEP8 checks > > > > On Mon, Jul 02, 2012 at 08:28:04AM -0400, Monty Taylor wrote: > > > > > > > > > On 07/02/2012 06:46 AM, John Garbutt wrote: > > > > Hi, > > > > > > > > I noticed I can now run the pep8 tests like this (taken from > > > > Jenkins job): > > > > tox -v -epep8 > > > > ... > > > > pep8: commands succeeded > > > > congratulations :) > > > > > > > > But the old way to run tests seems to fail: > > > > ./run-tests.sh -p > > > > ... > > > > File > > > > > "/home/johngar/openstack/nova/.venv/local/lib/python2.7/site-packages/pep8.py", > > > > line 1220, in check_logical > > > > for result in self.run_check(check, argument_names): > > > > TypeError: 'NoneType' object is not iterable > > > > > > > > Is this expected? > > > > Did I just miss an email about this change? > > > > > > I cannot reproduce this on my system. Can you run > > > "bash -x run_tests.sh -p" and pastebin the output? Also, > > > tools/with_venv.sh pep8 --version just to be sure. > > > > > > > Hi, > > > > The issue is that as of a recent change to upstream pep8 [1], the > > additional pep8 rules in tools/hacking.py need to be changed from > > returns to yields.. :( > > This brings up a good point. Why are we following the latest pep8 release > so closely in Nova? The latest release is hardly a month old and we are > already using it? We aren't necessarily doing the same for all the other > OpenStack projects... (nor am I suggesting that we do). So why Nova? > > I'm not convinced the latest pep8 "features" really provide us enough > benefit that we need to bump our pep8 baseline every month or two. in fact > they may be hurting us in terms of churn, extra work, back-portability of > upstream patches, etc. Ultimately is tracking the latest pep8 really worth > it? > +1 Some of the "features" are requiring useless whitespace changes to working code just to get through the gating tests. In ceilometer we've pegged our pep8 tests to 1.1. Doug > > > > > > [1] > > > https://github.com/jcrocholl/pep8/commit/b9f72b16011aac981ce9e3a47fd0ffb1d3d2e085 > > > > Kind Regards, > > > > Dave Walker > > Engineering Manager, > > Ubuntu Server Infrastructure > > > > ___ > > Mailing list: https://launchpad.net/~openstack > > Post to : openstack@lists.launchpad.net > > Unsubscribe : https://launchpad.net/~openstack > > More help : https://help.launchpad.net/ListHelp > > > > ___ > Mailing list: https://launchpad.net/~openstack > Post to : openstack@lists.launchpad.net > Unsubscribe : https://launchpad.net/~openstack > More help : https://help.launchpad.net/ListHelp > ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
Re: [Openstack] PEP8 checks
- Original Message - > From: "Dave Walker" > To: "Monty Taylor" > Cc: openstack@lists.launchpad.net, "John Garbutt" > Sent: Monday, July 9, 2012 6:01:19 PM > Subject: Re: [Openstack] PEP8 checks > > On Mon, Jul 02, 2012 at 08:28:04AM -0400, Monty Taylor wrote: > > > > > > On 07/02/2012 06:46 AM, John Garbutt wrote: > > > Hi, > > > > > > I noticed I can now run the pep8 tests like this (taken from > > > Jenkins job): > > > tox -v -epep8 > > > ... > > > pep8: commands succeeded > > > congratulations :) > > > > > > But the old way to run tests seems to fail: > > > ./run-tests.sh -p > > > ... > > > File > > > > > > "/home/johngar/openstack/nova/.venv/local/lib/python2.7/site-packages/pep8.py", > > > line 1220, in check_logical > > > for result in self.run_check(check, argument_names): > > > TypeError: 'NoneType' object is not iterable > > > > > > Is this expected? > > > Did I just miss an email about this change? > > > > I cannot reproduce this on my system. Can you run > > "bash -x run_tests.sh -p" and pastebin the output? Also, > > tools/with_venv.sh pep8 --version just to be sure. > > > > Hi, > > The issue is that as of a recent change to upstream pep8 [1], the > additional pep8 rules in tools/hacking.py need to be changed from > returns to yields.. :( This brings up a good point. Why are we following the latest pep8 release so closely in Nova? The latest release is hardly a month old and we are already using it? We aren't necessarily doing the same for all the other OpenStack projects... (nor am I suggesting that we do). So why Nova? I'm not convinced the latest pep8 "features" really provide us enough benefit that we need to bump our pep8 baseline every month or two. in fact they may be hurting us in terms of churn, extra work, back-portability of upstream patches, etc. Ultimately is tracking the latest pep8 really worth it? > > [1] > https://github.com/jcrocholl/pep8/commit/b9f72b16011aac981ce9e3a47fd0ffb1d3d2e085 > > Kind Regards, > > Dave Walker > Engineering Manager, > Ubuntu Server Infrastructure > > ___ > Mailing list: https://launchpad.net/~openstack > Post to : openstack@lists.launchpad.net > Unsubscribe : https://launchpad.net/~openstack > More help : https://help.launchpad.net/ListHelp > ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
Re: [Openstack] PEP8 checks
On Mon, Jul 9, 2012 at 3:01 PM, Dave Walker wrote: > On Mon, Jul 02, 2012 at 08:28:04AM -0400, Monty Taylor wrote: > > > > > > On 07/02/2012 06:46 AM, John Garbutt wrote: > > > Hi, > > > > > > I noticed I can now run the pep8 tests like this (taken from Jenkins > job): > > > tox -v -epep8 > > > ... > > > pep8: commands succeeded > > > congratulations :) > > > > > > But the old way to run tests seems to fail: > > > ./run-tests.sh -p > > > ... > > > File > "/home/johngar/openstack/nova/.venv/local/lib/python2.7/site-packages/pep8.py", > line 1220, in check_logical > > > for result in self.run_check(check, argument_names): > > > TypeError: 'NoneType' object is not iterable > > > > > > Is this expected? > > > Did I just miss an email about this change? > > > > I cannot reproduce this on my system. Can you run > > "bash -x run_tests.sh -p" and pastebin the output? Also, > > tools/with_venv.sh pep8 --version just to be sure. > > > > Hi, > > The issue is that as of a recent change to upstream pep8 [1], the > additional pep8 rules in tools/hacking.py need to be changed from > returns to yields.. :( > Proof of Concept: https://review.openstack.org/#/c/9569 > > [1] > https://github.com/jcrocholl/pep8/commit/b9f72b16011aac981ce9e3a47fd0ffb1d3d2e085 > > Kind Regards, > > Dave Walker > Engineering Manager, > Ubuntu Server Infrastructure > > ___ > Mailing list: https://launchpad.net/~openstack > Post to : openstack@lists.launchpad.net > Unsubscribe : https://launchpad.net/~openstack > More help : https://help.launchpad.net/ListHelp > > ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
Re: [Openstack] PEP8 checks
On Mon, Jul 02, 2012 at 08:28:04AM -0400, Monty Taylor wrote: > > > On 07/02/2012 06:46 AM, John Garbutt wrote: > > Hi, > > > > I noticed I can now run the pep8 tests like this (taken from Jenkins job): > > tox -v -epep8 > > ... > > pep8: commands succeeded > > congratulations :) > > > > But the old way to run tests seems to fail: > > ./run-tests.sh -p > > ... > > File > > "/home/johngar/openstack/nova/.venv/local/lib/python2.7/site-packages/pep8.py", > > line 1220, in check_logical > > for result in self.run_check(check, argument_names): > > TypeError: 'NoneType' object is not iterable > > > > Is this expected? > > Did I just miss an email about this change? > > I cannot reproduce this on my system. Can you run > "bash -x run_tests.sh -p" and pastebin the output? Also, > tools/with_venv.sh pep8 --version just to be sure. > Hi, The issue is that as of a recent change to upstream pep8 [1], the additional pep8 rules in tools/hacking.py need to be changed from returns to yields.. :( [1] https://github.com/jcrocholl/pep8/commit/b9f72b16011aac981ce9e3a47fd0ffb1d3d2e085 Kind Regards, Dave Walker Engineering Manager, Ubuntu Server Infrastructure signature.asc Description: Digital signature ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
Re: [Openstack] PEP8 checks
On Mon, 2012-07-02 at 08:15 -0400, Monty Taylor wrote: > It's not really expected, and I honestly don't understand why > run_tests.sh -p would have problems running pep8. Although we do not > use > run_tests.sh for anything in jenkins, we have not done anything to > disable or change what it's doing. I need to point out that "run_tests.sh -p" doesn't run straight-up pep8; it monkey-patches the pep8 tool to include several HACKING-compliance tests. Ever since tox stopped using this version of pep8, several HACKING-compliance issues have crept into the code base. -- Kevin L. Mitchell ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
Re: [Openstack] PEP8 checks
Awesome. On 07/02/2012 08:46 AM, John Garbutt wrote: > I had pep8 v1.2 in my virtual env from this: > https://github.com/openstack/nova/commit/2fa2cd2254a4044aaa584c4fcf5d6c3e1ec60d1f#tools/test-requires > > Rebased with trunk and re-creating my virtualenv (i.e. move back to pep8 > v1..1) seemed to fix things. > > Thanks for your help, > John > >> -Original Message- >> From: Monty Taylor [mailto:mord...@inaugust.com] >> Sent: 02 July 2012 1:28 >> To: John Garbutt >> Cc: openstack@lists.launchpad.net >> Subject: Re: [Openstack] PEP8 checks >> >> >> >> On 07/02/2012 06:46 AM, John Garbutt wrote: >>> Hi, >>> >>> I noticed I can now run the pep8 tests like this (taken from Jenkins job): >>> tox -v -epep8 >>> ... >>> pep8: commands succeeded >>> congratulations :) >>> >>> But the old way to run tests seems to fail: >>> ./run-tests.sh -p >>> ... >>> File >> "/home/johngar/openstack/nova/.venv/local/lib/python2.7/site- >> packages/pep8.py", line 1220, in check_logical >>> for result in self.run_check(check, argument_names): >>> TypeError: 'NoneType' object is not iterable >>> >>> Is this expected? >>> Did I just miss an email about this change? >> >> I cannot reproduce this on my system. Can you run "bash -x run_tests.sh -p" >> and pastebin the output? Also, tools/with_venv.sh pep8 --version just to be >> sure. > ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
Re: [Openstack] PEP8 checks
I had pep8 v1.2 in my virtual env from this: https://github.com/openstack/nova/commit/2fa2cd2254a4044aaa584c4fcf5d6c3e1ec60d1f#tools/test-requires Rebased with trunk and re-creating my virtualenv (i.e. move back to pep8 v1.1) seemed to fix things. Thanks for your help, John > -Original Message- > From: Monty Taylor [mailto:mord...@inaugust.com] > Sent: 02 July 2012 1:28 > To: John Garbutt > Cc: openstack@lists.launchpad.net > Subject: Re: [Openstack] PEP8 checks > > > > On 07/02/2012 06:46 AM, John Garbutt wrote: > > Hi, > > > > I noticed I can now run the pep8 tests like this (taken from Jenkins job): > > tox -v -epep8 > > ... > > pep8: commands succeeded > > congratulations :) > > > > But the old way to run tests seems to fail: > > ./run-tests.sh -p > > ... > > File > "/home/johngar/openstack/nova/.venv/local/lib/python2.7/site- > packages/pep8.py", line 1220, in check_logical > > for result in self.run_check(check, argument_names): > > TypeError: 'NoneType' object is not iterable > > > > Is this expected? > > Did I just miss an email about this change? > > I cannot reproduce this on my system. Can you run "bash -x run_tests.sh -p" > and pastebin the output? Also, tools/with_venv.sh pep8 --version just to be > sure. ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
Re: [Openstack] PEP8 checks
On 07/02/2012 06:46 AM, John Garbutt wrote: > Hi, > > I noticed I can now run the pep8 tests like this (taken from Jenkins job): > tox -v -epep8 > ... > pep8: commands succeeded > congratulations :) > > But the old way to run tests seems to fail: > ./run-tests.sh -p > ... > File > "/home/johngar/openstack/nova/.venv/local/lib/python2.7/site-packages/pep8.py", > line 1220, in check_logical > for result in self.run_check(check, argument_names): > TypeError: 'NoneType' object is not iterable > > Is this expected? > Did I just miss an email about this change? I cannot reproduce this on my system. Can you run "bash -x run_tests.sh -p" and pastebin the output? Also, tools/with_venv.sh pep8 --version just to be sure. ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
Re: [Openstack] PEP8 checks
On 07/02/2012 06:46 AM, John Garbutt wrote: > Hi, > > I noticed I can now run the pep8 tests like this (taken from Jenkins job): > tox -v -epep8 > ... > pep8: commands succeeded > congratulations :) > > But the old way to run tests seems to fail: > ./run-tests.sh -p > ... > File > "/home/johngar/openstack/nova/.venv/local/lib/python2.7/site-packages/pep8.py", > line 1220, in check_logical > for result in self.run_check(check, argument_names): > TypeError: 'NoneType' object is not iterable > > Is this expected? > Did I just miss an email about this change? It's not really expected, and I honestly don't understand why run_tests.sh -p would have problems running pep8. Although we do not use run_tests.sh for anything in jenkins, we have not done anything to disable or change what it's doing. I'm looking at it right now, lemme see what's up. Monty ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
[Openstack] PEP8 checks
Hi, I noticed I can now run the pep8 tests like this (taken from Jenkins job): tox -v -epep8 ... pep8: commands succeeded congratulations :) But the old way to run tests seems to fail: ./run-tests.sh -p ... File "/home/johngar/openstack/nova/.venv/local/lib/python2.7/site-packages/pep8.py", line 1220, in check_logical for result in self.run_check(check, argument_names): TypeError: 'NoneType' object is not iterable Is this expected? Did I just miss an email about this change? Cheers, John ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
