Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-06-14
15:28:20 dansmith efried: that's not as easy
15:28:27 cdent i'm basically done
15:28:27 dansmith because we calculate column names in places
15:28:30 cdent yes, that
15:29:32 efried right, I get that, so I'm fine with it. But I actually prefer the explicitness of takashi's patch. But I'm fine either way.
15:30:14 cdent the new diff is way tidier
15:30:16 dansmith joinedload, joinedload_all, joined, etc has brought plenty of confusion in the past, so more consistency for people to copy seems like a win to me
15:30:56 dansmith also, cdent and I are agreeing which means (1) jump on that shit and (2) go buy some lottery tickets
15:31:08 cdent ✔
15:32:32 openstackgerrit Chris Dent proposed openstack/nova master: Replace joinedload_all with joinedload https://review.opendev.org/664193
15:32:49 cdent i didn't run any tests on that ^
15:32:54 cdent will start them now
15:33:52 dansmith cdent: I was going to write a unit test for the helper too
15:34:20 efried lgtm cdent
15:34:41 cdent yeah, I gotta run now, so hope that's suitable, but will leave it in dan's earlier in his day hands
15:38:58 openstackgerrit Stephen Finucane proposed openstack/nova master: Follow-up for I6a777b4b7a5729488f939df8c40e49bd40aec3dd https://review.opendev.org/664967
15:38:58 openstackgerrit Stephen Finucane proposed openstack/nova master: Remove 'MultiattachSupportNotYetAvailable' exception https://review.opendev.org/651315
15:39:35 finucannot efried: I removed the additional test that I'd meant to remove in https://review.opendev.org/#/c/664967/. Fancy revisiting?
15:39:47 efried ...
15:40:26 finucannot efried: Addressed this comment https://review.opendev.org/#/c/621061/8/nova/tests/unit/compute/test_compute_api.py@4477
15:40:35 finucannot in the follow-up
15:41:26 openstackgerrit Stephen Finucane proposed openstack/nova master: objects: Remove legacy '_from_dict' functions https://review.opendev.org/537414
15:41:33 efried finucannot: +2, thanks for getting on the new=mock.Mock() bandwagon with me.
15:48:17 dansmith efried: you have insulted me to my core
15:48:29 efried payback is hell
15:49:59 sean-k-mooney efried: why use new=mock.mock() ? in what case is that needed?
15:50:33 dansmith new= prevents it from being passed as an arg
15:50:51 efried sean-k-mooney: When using decorator:
15:50:51 efried @mock.patch("...") <== causes an argument to be created to your method
15:50:51 efried @mock.patch("...", new=...) <== no arg
15:50:54 efried yeah, what dansmith said
15:51:10 sean-k-mooney dansmith: oh ok that makes sense if you never want to use of check it
15:51:12 temka efried, that's *massively* useful
15:51:22 temka efried, I've been using _ where possible
15:51:36 dansmith sean-k-mooney: yeah, but it's also a good thing to verify that the person isn't skirting some checking they should be doing as a result :)
15:51:42 sean-k-mooney temka: in a fucntion signiture
15:51:46 efried temka: You mean it's a thing you didn't know, or you're agreeing that it's useful and you use it?
15:51:55 temka efried, yes ;)
15:51:56 sean-k-mooney efried: yes
15:52:08 temka sean-k-mooney, yeah, you can have def blah(self, _)
15:52:23 sean-k-mooney temka: yes that would be an instance -1 form me
15:52:33 temka sean-k-mooney, you'd be alone then :)
15:52:36 sean-k-mooney i really think that is poor api design
15:52:45 temka Only in unit tests
15:53:08 sean-k-mooney you are technically not overloading if you cange teh name to _
15:53:25 sean-k-mooney so even in unit tests you shoudl not do that
15:53:45 temka [artom@zoe nova]$ grep def.*_\) -R nova/tests/ | wc -l
15:53:45 temka 13
15:53:49 sean-k-mooney i guess you are doingit for function that will be used in place of a mock?
15:53:50 temka Wonder if all of those are mine
15:54:30 openstackgerrit Dan Smith proposed openstack/nova master: Add a test for the _joinedload_all helper https://review.opendev.org/665421
15:54:31 dansmith efried: ^
15:55:50 efried dansmith: +2, nice. Added cdent
15:56:08 sean-k-mooney temka: i wonder if the new=mock.Mock works the same way when your apply the decorator to a class
15:56:17 efried it does
15:56:30 sean-k-mooney normally that results in the moc being passed to all fucntion or in this case not being based
15:56:34 sean-k-mooney passed
15:58:17 sean-k-mooney is mriedemn around today
15:59:11 sean-k-mooney mriedemn: just droping off for the day but i adress your comments in https://review.opendev.org/#/c/647733/
15:59:23 dansmith sean-k-mooney: he's out today
15:59:30 sean-k-mooney ok no worries
15:59:43 sean-k-mooney i was just going to say if he spots anythin else ill adress it on monday
16:03:15 finucannot melwitt: Should I drag https://review.opendev.org/#/c/652967/ to the front of the series and add a bug link?
16:03:32 finucannot i.e. do we care enough about the XVP VNC proxy to both backporting?
16:03:48 finucannot *bother
16:04:24 melwitt finucannot: I was debating about it... because yeah it's a bug but we have like zero testing for this so it would have to be a testless bug fix else you'd have to figure out how to add testing for a thing no one's using. so I dunno
16:06:40 finucannot Yeah, we have zero tests for that and can probably remove it this cycle
16:06:51 melwitt oh and I would not expect to backport
16:07:08 finucannot Okay, then I don't need to worry about opening a bug for this so
16:07:44 finucannot BobBall said on the ML some time back that they're not using it anymore
16:11:04 melwitt yeah. I'd just leave it for now and see if mriedem complains
16:11:38 melwitt he's the expert on these type of complicated matters
16:12:09 sean-k-mooney mtreinish: tonyb[m] would ye be able to take a look at these trivial test only stable backport for me on os-vif. https://review.opendev.org/#/c/665196/ and https://review.opendev.org/#/c/665195/
16:17:58 openstackgerrit Stephen Finucane proposed openstack/nova master: xvp: Start using consoleauth tokens https://review.opendev.org/652967
16:17:58 openstackgerrit Stephen Finucane proposed openstack/nova master: nova-status: Remove consoleauth workaround check https://review.opendev.org/652968
16:17:59 openstackgerrit Stephen Finucane proposed openstack/nova master: Remove nova-consoleauth https://review.opendev.org/652969
16:17:59 openstackgerrit Stephen Finucane proposed openstack/nova master: objects: Remove ConsoleAuthToken.to_dict https://review.opendev.org/652970
16:18:00 openstackgerrit Stephen Finucane proposed openstack/nova master: docs: Rework nova console diagram https://review.opendev.org/660147
16:18:00 openstackgerrit Stephen Finucane proposed openstack/nova master: xvp: Remove use of '_LI' marker https://review.opendev.org/665425
16:36:09 finucannot argparse is broken in so many little ways that it kind of hurts :(
16:37:02 temka moar liek arseparse, amirite?
16:37:15 finucannot temka++
16:38:17 cdent temka+++
16:38:29 cdent argparse is so very very ugh
16:40:56 finucannot It tries to be too clever
16:41:01 finucannot and fails miserably
16:41:52 finucannot fwiw, click (which is A+) is based on optparse and not argparse for just that reason
16:44:48 openstackgerrit Stephen Finucane proposed openstack/nova master: Always Set dhcp_server in network_info https://review.opendev.org/658362
16:47:23 efried finucannot: https://review.opendev.org/#/c/664396/ - missed one --?
16:48:33 finucannot my bad. On it
16:49:47 openstackgerrit Stephen Finucane proposed openstack/nova master: docs: Document how to revert, confirm a cold migration https://review.opendev.org/664396
16:49:48 openstackgerrit Stephen Finucane proposed openstack/nova master: docs: Update resize doc https://review.opendev.org/665412
16:50:03 finucannot efried: ^
16:51:18 efried finucannot: thx, +2
16:53:18 sean-k-mooney am i missing somthing
16:53:37 sean-k-mooney "openstack server migrate revert" is not a thing
16:54:03 sean-k-mooney nor is "openstack server migrate confirm"
16:54:42 sean-k-mooney you have to use openstack server resize --revert or openstack server resize --confirm
16:55:41 sean-k-mooney oh your adding them in the depnds on patch
16:56:46 openstackgerrit Stephen Finucane proposed openstack/nova master: This patch is to fix bug #1831657 https://review.opendev.org/663926

Earlier   Later