Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2023-02-14
16:40:40 bauzas but AFAIR, we only enforce line lengths on code
16:40:55 bauzas that has to be doublechecked
16:41:03 bauzas anyway
16:41:15 bauzas let's wrap it now and chase this question outside of the meeting
16:41:17 bauzas thanks all
16:41:22 bauzas #endmeeting
16:41:23 opendevmeet Meeting ended Tue Feb 14 16:41:22 2023 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
16:41:23 opendevmeet Minutes: https://meetings.opendev.org/meetings/nova/2023/nova.2023-02-14-16.00.html
16:41:23 opendevmeet Minutes (text): https://meetings.opendev.org/meetings/nova/2023/nova.2023-02-14-16.00.txt
16:41:23 opendevmeet Log: https://meetings.opendev.org/meetings/nova/2023/nova.2023-02-14-16.00.log.html
16:41:27 bauzas so
16:41:42 bauzas we now enforce the line lengths by a pre-commit
16:42:02 bauzas not sure our tox pep8 target continues to check it
16:42:23 artom I only noticed it because a downstream pep8 job failed
16:42:29 artom It's from a commit in 2020
16:42:32 artom https://review.opendev.org/c/openstack/nova/+/738738
16:42:32 bauzas oh
16:42:36 bauzas no, I'm wrong
16:42:46 bauzas we autopep8 it with the pep8 target
16:43:08 bauzas https://github.com/openstack/nova/blob/master/tox.ini#L100
16:45:56 clarkb bauzas: the --inplace flag doesn't appear to be a currently documented flag. Maybe it is fixing your code in CI and the flake8 passes after
16:46:29 bauzas clarkb: that's my guess but that doesn't explain this https://opendev.org/openstack/nova/src/branch/master/nova/image/glance.py#L392
16:50:53 clarkb looking at the flake8 script it passes in the target posargs as the arguments to flake8
16:51:30 clarkb the CI jobs don't have posargs by default (you could override them though I think, but I'm not seeing that in logs). Does flake8 check things without args?
16:51:38 clarkb you might simply be not checking things? I dunno
16:53:47 gibi I just tried, flake8 check all files if called without args and it finds the long line I add in nova/image/glance.py but not the L392
16:54:50 clarkb autopep8 relies on pycodestyle to know whento change things too. Maybe the issue is in pycodestyle then where it doesn't see that as a problem so neither autopep8 nor flake8 complain
16:54:56 artom Yeah, same here. And it's not a comment thing (at least with # ) because if I comment out the long line it still finds it
16:55:34 gibi artom: yeah, I added a long line in the same doc comment and it finds that
16:56:03 artom Ah, but apparently not the *first* line?
16:56:41 artom So yeah
16:56:42 artom def _get_verifier(self, context, image_id, trusted_certs):
16:56:42 artom """Really long line long long long long long long long long long long long
16:56:42 artom Really long line long long long long long long long long long long long long long
16:56:42 artom """
16:56:51 artom It complains about the second line (without """)
16:56:54 artom But not the first line
16:57:17 gibi yeah it seems the leading line is ignored
16:57:36 clarkb is the threshold different? I wonder if it complains eventually
16:57:40 gibi I moved L392 to a new line and padded it with 3 leading char and it finds it
16:58:33 gibi yeah if I pad the leading line with 10 extra chars then it finds it
16:58:38 artom Is that a bug in hacking or pep8? As I said, a downstream pep8 check does find it, using an older version of hacking/pep8 I think
16:59:06 clarkb artom: considering that autopep8 which also relies on pycodestyle doesn't complain about it either probably a thing in pycodestyle
16:59:09 artom Ah, so it looks like it ignores the leading """ or something?
17:00:30 gibi I have to add 6 or more chars to get a failure
17:00:43 gibi and when I get the failure it says line too long (80 > 79 characters)
17:00:54 gibi so it is counting the charachters wrongly
17:01:16 gibi as at that point the line is 88 chars long
17:03:18 gibi what else will break on us this week?!?!
17:03:41 artom Presumably that's been broken for a while, and it's minor
17:05:02 artom Ah https://github.com/PyCQA/pycodestyle/issues/679
17:05:35 artom Which leads down the rabbit hole https://github.com/PyCQA/flake8/issues/1534
17:06:00 artom Seems like it should be fixed in flake8 5.0.0?
17:10:09 gibi nice
17:10:12 artom Ah, I think our flake8 is capped by our hacking version
17:10:20 artom hacking>=3.1.0,<3.2.0 # Apache-2.0
17:10:23 artom From test-requirements
17:11:45 gibi that will be a "nice" bump to make
17:12:04 artom I imagine there's a reason that it was there in the first place?
17:12:37 artom All I can find is https://review.opendev.org/c/openstack/nova/+/727589
17:12:51 artom Which appears to just decide that 3.2.0 is the max for some reason
17:17:50 gibi even if we bump hacking to the maximum we only get to flake8 4.0 https://opendev.org/openstack/hacking/src/branch/master/requirements.txt#L1
17:19:08 artom Huh, so why is that capped
17:21:23 artom No documented reason that I can see, even going back as far as e664ef421c60ade4c2557e8d7029b81ccb8478a0
17:21:34 opendevreview Sylvain Bauza proposed openstack/nova master: Revert "Add logging to find test cases leaking libvirt threads" https://review.opendev.org/c/openstack/nova/+/873584
17:23:11 bauzas gibi: artom: sorry, I got distracted by other embargoed things
17:23:29 gibi artom: with hacking 5.0.0 (max) we have couple of findings https://paste.opendev.org/show/bQD8LQ9tmyPFD0NouJnv/ but nothing major
17:23:38 bauzas so yeah, like I said, I think the linter doesn't check the line length on a docstring
17:24:46 artom gibi, ok, but flake8 is still stuck on 4.0.1, so it doesn't catch the line length thing
17:25:26 artom I don't feel like I know enough about the release sausage to propose an increase in the cap of flake8 and hacking
17:25:36 bauzas gibi: it doesn't block our gate, does it ?
17:25:51 artom It feels like we should, though...
17:25:56 artom bauzas, no, it's cosmetic and minor
17:26:00 bauzas I'm very afraid of bumping our hacking requirements so close to the holy FF
17:26:27 artom bauzas, bumping *our* hacking requirement wouldn't be enough
17:26:36 artom We'd need to bump *hacking's* flake8 cap
17:26:47 bauzas I see
17:26:49 artom Which sounds even worse
17:26:59 bauzas if it's cosmetic, then you have MHO
17:27:14 bauzas probably better to just change the docstring
17:27:21 gibi artom: bumping hacking to flake8 5.0.4 causes unit test failures in hacking :/
17:27:41 artom Wow, wtf
17:27:44 bauzas but if that doesn't cause any harm, please defer it to Bobcat
17:28:29 gibi hold on, that might be not due to flake8 5.0.4
17:28:41 gibi bauzas: don't worry we won't bump hacking now :)
17:29:15 bauzas I mean, another library upgrade and then I get a heartbroke
17:30:10 gibi yeah the unit test of hacking fails on me on master too :/
17:30:33 artom Err
17:30:47 artom I guess stuff changed, and the unit tests job just never ran?
17:31:00 gibi anyhow I think the whole 1) bump hacking to use flake8 5.0 3) release a new hacking 2) bump nova to use latest hacking. Is doable probably.
17:31:08 gibi artom: or my local env is bork
17:33:09 gibi we will see https://review.opendev.org/c/openstack/hacking/+/873737
17:35:04 gibi and I'm feeling lucky https://review.opendev.org/c/openstack/hacking/+/873738
17:37:47 artom You absolute madlad
17:39:09 gibi I don't know what was in my afternoon coffee but I feel like a squirrel on cocain
17:39:40 artom Well you just answered your own questions. You coffee contained squirrels. And cocaine.
17:39:45 gibi :D
17:40:30 gibi interestingly it is from the same batch of beans that I used in the last couple of weeks without such effect.
17:41:08 artom So obviously this morning a squirrel decided to use it to stash its cocaine.
17:43:26 gibi yepp the hacking unit test on master fails in CI too https://13105f8ef823650ec019-cb65abe58d87a1a010092a9adcbaff91.ssl.cf2.rackcdn.com/873737/1/check/openstack-tox-py38/9f6af81/testr_results.html

Earlier   Later