Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-05-14
07:50:26 brinzhang_ morning nova
07:50:59 brinzhang_ hi gibi, bauzas, I saw you are -1 for https://review.opendev.org/#/c/727140/1, the flake8 error fix patch
07:51:44 gibi brinzhang_: my -1 is about the fact that 3.8.1 added yet another check that needs to be fixed
07:51:48 brinzhang_ I submit a same one, before that I have not concern stephenfin's patch, do I need to continue ?
07:52:24 brinzhang_ gibi: I will see http://paste.openstack.org/show/793434/
07:52:40 bauzas brinzhang_: my concerns was about making sure what we would need to change
07:52:49 gibi I guess stephenfin abandoned the patch as we went a different direction, capping the version
07:53:34 brinzhang_ yeah
07:53:55 brinzhang_ I mean, if there is no sense to fix, I think I would abandon too.
07:56:16 brinzhang_ https://review.opendev.org/#/c/727347/ gmann bumped hacking to 3.0.1, I am not sure whether this can avoid these pep8 error happen again
07:58:30 brinzhang_ gibi: stephenfin and I fixed the issue is reported by https://review.opendev.org/#/c/724260/6, and https://review.opendev.org/#/c/726564/2.
07:59:12 brinzhang_ I found your paste is not easy to fix, may need to change the function name, such as "./nova/tests/unit/api/openstack/compute/microversions.py:34:5: F811 redefinition of unused 'index' from line 29"
07:59:18 brinzhang_ :P
08:02:05 gibi brinzhang_: those redefinitions are intentional and not cause a python bug due to the decorator top of it will generate unique functions dynamically. So I think F811 needs to be ignored in tox.ini
08:03:01 gibi I think it is OK to fix the issues reported by the other new checks along with version bump
08:03:09 gibi but please consult stephenfin too
08:03:54 brinzhang_ gibi: agree
08:04:49 brinzhang_ gibi: yeah, I will try to update the patch to latest, and then consult
08:05:32 gibi OK, cool
08:20:58 openstackgerrit Brin Zhang proposed openstack/nova master: Fix flake8 error from hacking <3.0.1 https://review.opendev.org/727589
08:45:27 bauzas brinzhang_: gibi: FWIW, +Wd https://review.opendev.org/#/c/727347/1
08:45:57 bauzas brinzhang_: but then I don't understand why we should merge https://review.opendev.org/#/c/727589/
08:46:04 gibi bauzas: thanks
08:50:36 bauzas gibi: any change I should be reviewing ?
08:51:23 openstack Launchpad bug 1878024 in OpenStack Compute (nova) "disk usage of the nova image cache is not counted as used disk space" [Undecided,Confirmed] - Assigned to Balazs Gibizer (balazs-gibizer)
08:51:23 gibi bauzas: not from me at the moment. I still busy with https://bugs.launchpad.net/nova/+bug/1878024
08:51:41 bauzas gibi: okay, just saying, you have https://review.opendev.org/#/q/topic:bug/1869050+(status:open+OR+status:merged)
08:51:51 bauzas should I look at it ?
08:52:13 gibi bauzas: yes please. I already forgot about that
08:52:23 bauzas heh, no worries
09:00:45 brinzhang_ bauzas: https://review.opendev.org/#/c/727589/ this patch just only fixed some unreasonable issue, IMO, if we don’t fix it, we don’t know when it will *explode*
09:01:08 brinzhang_ after all, it has reported in https://review.opendev.org/#/c/724260/6, and https://review.opendev.org/#/c/726564/2.
09:11:09 bauzas brinzhang_: that's the exact reason why we have minimum versions
09:11:41 bauzas once we bump the minimum, we no longer need to care about what was bad for older versions
09:12:23 brinzhang_ bauzas: I know you mean, but these code realy not have a good code style, right?
09:13:41 brinzhang_ I can say with certainty that merged this change will only make Nova's code more readable.
09:14:37 bauzas then that's not a bugfix
09:14:42 brinzhang_ especially https://review.opendev.org/#/c/727589/3/nova/tests/unit/compute/test_compute_mgr.py this patch
09:14:49 brinzhang_ s/patch/file/
09:14:57 bauzas and just a bikeshed
09:15:23 brinzhang_ if there is no this blank line, it..
09:16:09 brinzhang_ Yes, this bug was submitted before https://review.opendev.org/#/c/727347/1
09:16:58 brinzhang_ because it caused pep8 error in https://review.opendev.org/#/c/724260/6, and https://review.opendev.org/#/c/726564/2.
09:17:26 brinzhang_ that time I think it is a bug, that why report
09:19:18 sean-k-mooney bauzas: we will eventually have to fix the pep8 issues by the way when we update hacking
09:19:36 sean-k-mooney and therefor get the newer flake8
09:20:02 bauzas again, we have minimum versions FTW
09:20:16 sean-k-mooney yep wont help
09:20:43 bauzas the point is, if we want to support new flake8 version, then yes indeed we need to verify this works
09:20:46 sean-k-mooney the new min version of hacking is so we get a version that caps flake8 and we have capped the hacking version
09:20:50 bauzas and then updating what we need
09:21:07 sean-k-mooney so when we update the hacking version we will get a new flake8 and have to fix the issue then
09:21:09 bauzas but that's another concern
09:21:22 bauzas correct, that's another change
09:21:39 bauzas anyway, /me needs to get my kid from the school since it opened again today :(
09:22:00 bauzas (until next kids vacations)
09:22:12 sean-k-mooney i would have assumed your productivig increased with them going back to school
09:22:31 sean-k-mooney so in net it should still be better
09:23:03 sean-k-mooney even if its about to go down now that you are going to pick them up
09:25:17 brinzhang_ Agree, a good foundation can make a difference.
09:42:32 bauzas sean-k-mooney: hell, unfortunately no
09:42:48 bauzas sean-k-mooney: I was working between 9am-1pm and then 2pm-7.30pm
09:42:55 bauzas even with the kids
09:43:07 bauzas they were asking me for teaching only for 1 hour
09:43:30 bauzas now, it's 9am-11.30am and then 1.30pm-4.30pm
09:43:39 bauzas :(
09:53:01 openstackgerrit sean mooney proposed openstack/nova master: cyborg evacuate support https://review.opendev.org/715326
09:55:27 sean-k-mooney gibi: brinzhang_ filed https://blueprints.launchpad.net/nova/+spec/cyborg-rebuild-and-evacuate
09:56:09 sean-k-mooney ill add it to the meeting adgenda but im hoping we are all fine with proceeding with that as a specless blueprint since i have already implemented it
09:57:09 gibi sean-k-mooney: thanks
09:58:55 sean-k-mooney bauzas: im going to update the numa in placmenet spec for the victoria templeate and resubmit it now. that ok with you?
09:59:34 brinzhang_ sean-k-mooney: got it, thanks, I will register suspend/resume and shelve/unshelve as the same rebuild/evacuate
10:00:11 brinzhang_ sean-k-mooney, gibi: and there is also another blueprint need to be done, https://blueprints.launchpad.net/nova/+spec/define-cyborg-arq-binding-notification-event
10:01:09 gibi stephenfin: do you mean merging https://review.opendev.org/#/c/727898/ is not needed if we merging https://review.opendev.org/#/c/727898/ and doing a version bump?
10:01:13 brinzhang_ I write poc code, but there is not time to add UT, when I have free time, I will summit a WIP patch for this
10:01:45 gibi stephenfin: I mean merging your patch https://review.opendev.org/#/c/727992/
10:03:58 gibi brinzhang_: could you write one sentence in that bp about what you need implemented?
10:04:50 brinzhang_ gibi: ok
10:04:53 gibi thanks
10:07:37 stephenfin gibi: Yeah, exactly
10:08:29 gibi stephenfin: if we can merge that quickly then I'm OK to wait for the better solution
10:09:06 openstackgerrit sean mooney proposed openstack/nova-specs master: re-propose numa with placement https://review.opendev.org/728009
10:09:42 brinzhang_ gibi: I modified https://blueprints.launchpad.net/nova/+spec/define-cyborg-arq-binding-notification-event description in simple, is it ok?
10:11:35 gibi brinzhang_: what you have to add regarding "accelerators binding notifications." ?
10:11:44 gibi what you have to add to nova
10:12:41 sean-k-mooney gmann: when you have time could you weigh in on https://review.opendev.org/#/c/702394/ i think it likely should have a specless blueprint or a spec since its technicall changing api behavior by cause the event api to force refresh the netwrok info cache. that is an internal detail however so im not sure it should have a micro version bump as form a normal user perspecitive that should not be visable
10:12:43 sean-k-mooney but it would be good to get your input.
10:16:18 brinzhang_ gibi: updated, I think it's the clear I can write in
10:16:36 brinzhang_ stephenfin: what do you think of https://review.opendev.org/#/c/727589/ ?
10:17:16 brinzhang_ I saw you were abandoned https://review.opendev.org/#/c/727140/, would you like to continue that patch?
10:18:23 openstackgerrit Lee Yarwood proposed openstack/nova master: compute: Remove snapshot quiesce tests for STOPPED and SUSPENDED instances https://review.opendev.org/728012
10:18:23 openstackgerrit Lee Yarwood proposed openstack/nova master: compute: Allow snapshots to be created from PAUSED volume backed instances https://review.opendev.org/728011
10:18:47 lyarwood ^ simple fix if anyone has time
10:18:50 stephenfin brinzhang_: hacking 3.1.0 will bump the version of flake8 supported so it would probably make more sense to wait for that to be released
10:20:13 stephenfin brinzhang_: I'm okay for you to take over that work
10:20:16 brinzhang_ yeah, I know, I mean do we need to correct these code style, after we bump hacking>=3.0.1
10:21:02 gibi brinzhang_: I'm still confused. Nova does not have versioned notification for neutron port binding during nova server create. So I don't see why nova would emit versioned notifications for ARQ binding
10:21:31 brinzhang_ stephenfin: thanks, I want to say when I submit this patch, I have not check your patch
10:23:12 openstack Launchpad bug 1865549 in OpenStack Compute (nova) " Define Cyborg ARQ binding notification event." [Wishlist,Confirmed] - Assigned to Brin Zhang (zhangbailin)
10:23:12 brinzhang_ gibi: this Blueprint comes from https://bugs.launchpad.net/nova/+bug/1865549, when I saw this bug, I just only think define-cyborg-arq-binding-notification-event is what I should do, or I missed something?

Earlier   Later