| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-23 | |||
| 12:40:19 | cdent | johnthetubaguy, bauzas, jaypipes : that's ^ probably reasonably important enough to merge soonish | |
| 12:44:43 | sbezverk_ | cdent: do you have a second for qq? | |
| 12:45:07 | cdent | sbezverk_: I can try | |
| 12:46:14 | sbezverk_ | cdent: in ocata, the doc sais that we need to have two cells cell0 and cell1, but on the other hand I am running ocata with just cell0 and everything is working. SO there is an inconsistency. Could you please clarify? | |
| 12:46:15 | tojuvone | johnthetubaguy, No problem :) | |
| 12:46:37 | sdague | cdent: could you put a test that tries to set the decorator with a float to ensure that explodes really early? | |
| 12:46:44 | cdent | sdague: sure | |
| 12:46:55 | sdague | because I would be concerned by someone doing that accidentally | |
| 12:47:07 | sbezverk_ | cdent: is it really mandatory to have both cells? | |
| 12:47:24 | Dinesh_Bhor | Hi all, can someone take a look at a small patch? https://review.openstack.org/#/c/445782/ It's already having a +2. | |
| 12:47:34 | cdent | sbezverk_: As I understand it, yes, so something is likely not what it appears, but that's an area I'm not very familiar with | |
| 12:48:02 | sbezverk_ | cdent: could you suggest local cell expert ;) ?? | |
| 12:48:59 | sbezverk_ | cdent: cause as I said I have a test bed functioning normal with single cell0. | |
| 12:49:18 | cdent | sbezverk_: dansmith is probably the final word there and melwitt as well but I think neither of them is around yet | |
| 12:49:48 | EmilienM | jroll: fyi, the ironic/nova thing is fixed for us. Thanks again for your patch! | |
| 12:49:55 | cdent | sbezverk_: are you sure you haven't named the thing which would be cell1 'cell0'? I believe you should have three database tables? | |
| 12:49:58 | EmilienM | dansmith, bauzas ^ | |
| 12:50:20 | bauzas | EmilienM: coolio | |
| 12:50:41 | sbezverk_ | cdent: positive, I wrote a job and it creates a single cell0 | |
| 12:51:15 | sbezverk_ | cdent: so I have now 3 databases only nova, nova-api and cell0 | |
| 12:51:35 | sbezverk_ | cdent: I will follow up with these folks to confirm, thank you very much for your help | |
| 12:52:02 | cdent | sbezverk_: I think what you've got there is correct | |
| 12:52:25 | cdent | but like I said, I've not done much poking in this area, and I don't currently have a running system | |
| 12:52:56 | cdent | sbezverk_: actually, i do have database tables though and I have 'nova', 'nova_api' and 'nova_cell0'. only | |
| 12:53:13 | cdent | the table named 'nova' is cell1 | |
| 12:54:07 | sbezverk_ | cdent: ahhh, so nova became kind of cell1, got it | |
| 12:54:40 | bauzas | cdent: https://review.openstack.org/#/c/449119/1 had sdague's comment that I agree with | |
| 12:54:50 | cdent | bauzas: yeah, just working on that now, thanks. | |
| 12:54:51 | bauzas | just add this and I'm +2 | |
| 12:55:09 | bauzas | cdent: FWIW, thanks for fixing that | |
| 13:02:10 | jroll | EmilienM: \o/ | |
| 13:06:26 | cdent | sdague, bauzas: If you pass in a float on that thing it will raise an AtributeError, at import time. Is that sufficient, or would you prefer that exception(s) raised from parse_version_string be more custom? I tend to prefer built in exceptions for this kind of thing. | |
| 13:07:16 | bauzas | cdent: I think sdague asked for an unittest verifying that behaviour | |
| 13:07:26 | cdent | yes, i've written the test | |
| 13:07:31 | bauzas | not what happens if someone uses that deco incorrectly | |
| 13:07:58 | bauzas | cdent: tbc, the former is enough from my PoV | |
| 13:08:20 | bauzas | if we have a docstring that explains the deco is awaiting for a string, and then the developer is using it incorrectly, then meh | |
| 13:09:08 | bauzas | it's like not following a manual when using a chainsaw | |
| 13:11:28 | sdague | cdent: as long as that would fail a commit I'm good with it | |
| 13:11:46 | cdent | sdague: yeah, code will boom | |
| 13:11:53 | sdague | cdent: wfm | |
| 13:13:31 | cdent | sdague: yeah: AttributeError: 'float' object has no attribute 'split'\nThe test run didn't actually run any tests" | |
| 13:13:48 | cdent | (that's on the functional tests) | |
| 13:13:53 | sdague | cool | |
| 13:22:34 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Don't use floats in microversion handling https://review.openstack.org/449119 | |
| 13:27:23 | cdent | sdague, bauzas: that ^ should do it | |
| 13:29:32 | sdague | cdent: frickler has a comment on the last patch | |
| 13:31:29 | bauzas | sdague: frickler missed the transformation into a tuple | |
| 13:32:35 | cdent | yeah, it's a tuple by then | |
| 13:33:05 | sdague | so, is there a reason not to move APIVersionRequest into the common library and reuse all of that? | |
| 13:33:59 | cdent | sdague: no reason other than time | |
| 13:34:56 | cdent | sdague: I also thought that writing something fresh would allow for some compare and contrast so we could have a mix of both in whatever the common thing was | |
| 13:35:08 | cdent | there are some differences in approach | |
| 13:35:12 | cdent | (nothing major) | |
| 13:35:23 | sdague | yeh, it feels like mostly we're just rediscovering all the bugs again :) | |
| 13:35:27 | cdent | heh | |
| 13:35:48 | sdague | I honestly remember this exact issue when we hit 2.10 | |
| 13:36:03 | efried | lifeless Yes, https://github.com/testing-cabal/mock/issues/398 is me. | |
| 13:36:47 | efried | mriedem I've got a doc appt during the nova meeting today. I left my update in the agenda. | |
| 13:38:11 | cdent | sdague: sure. the other two issues were: didn't want to reuse nova code because I thought we'd be moving soon; and when I first did this I couldn't seem to untangle the way in which nova was doing version_handler from other parts of the wsgi stack. | |
| 13:38:30 | cdent | In other words I found the code too hard to read, so wrote some more. Which is sad but true. | |
| 13:39:47 | cdent | (it's the stuff in Controller that bit me) | |
| 13:44:25 | mriedem | efried: ok | |
| 13:46:16 | sdague | cdent: ok, but you've effectively replicated the APIVersionRequest object as a tuple now. It feels like that's going to be a pretty direct transition | |
| 13:46:37 | openstackgerrit | Eric Fried proposed openstack/nova master: PowerVM Driver: spawn/delete #1: no-ops https://review.openstack.org/438119 | |
| 13:46:47 | openstackgerrit | Hironori Shiina proposed openstack/nova-specs master: Ironic: Cold migration support https://review.openstack.org/449155 | |
| 13:47:31 | cdent | sdague; that part is not the interesting/useful/verbose part of the handling though. It is the users of the Version concept, and those parts between placement and nova are much different (simply because the framework is too) | |
| 13:47:44 | cdent | I agree with you that there's some duplication, and that's unfortunate, but it's where we are right now | |
| 13:48:12 | cdent | and I'm not too het up about repeating bug fixes, it is good for learning, for refactoring, and refreshing, especially when they are caught before they leak | |
| 13:48:42 | cdent | we had the review process for all this work, and it missed some things, that's okay, that's life. | |
| 14:00:11 | mriedem | nova meeting time | |
| 14:11:27 | moshele | sean-k-mooney: hi, can you review the https://review.openstack.org/#/c/398277/ | |
| 14:18:15 | openstackgerrit | Gábor Antal proposed openstack/nova master: Transform instance.volume_attach notification https://review.openstack.org/401992 | |
| 14:18:16 | openstackgerrit | Gábor Antal proposed openstack/nova master: Transform instance.volume_detach notification https://review.openstack.org/408676 | |
| 14:19:26 | jgriffith | lyarwood sorry, I thought you were indicating that your patches that replaced mine had merged; still waiting before refactoring everything | |
| 14:20:12 | jgriffith | lyarwood looks like there's still some dialogue to hash out | |
| 14:21:00 | johnthetubaguy | I think lyarwood is out for a week, not 100% sure | |
| 14:22:10 | openstackgerrit | Gábor Antal proposed openstack/nova master: Transform instance.volume_attach notification https://review.openstack.org/401992 | |
| 14:55:19 | sdague | where are servers assigned to server_groups? | |
| 14:56:02 | dansmith | sdague: hint group=foo | |
| 14:56:54 | sdague | oh, it's buried inside hints? | |
| 14:57:32 | gibi | mriedem: during the nova meeting I got the info from sjmc7 about the needed transformations | |
| 14:57:47 | sdague | which aren't documented... that's the awesome :) | |
| 14:58:30 | gibi | mriedem: < sjmc7> gibi: the only notification we rely on that's not implemented is compute.instance.volume.attach/detach | |
| 14:58:54 | gibi | mriedem: so we are on a good track as those notifications already in focus | |
| 14:59:47 | openstack | Launchpad bug 1675276 in OpenStack Compute (nova) "Volumes attached to shelved instance may contain incorrect device_name" [Undecided,In progress] - Assigned to Zhenyu Zheng (zhengzhenyu) | |
| 14:59:47 | Kevin_Zheng | https://bugs.launchpad.net/nova/+bug/1675276 | |
| 15:00:25 | mriedem | Kevin_Zheng: the device_name is never guaranteed | |
| 15:00:39 | dansmith | sdague: yep, create a group foo, then boot an instance with hint group=foo | |
| 15:00:39 | bauzas | sdague: yup | |
| 15:00:43 | mriedem | Kevin_Zheng: we've talked about just removing the device_name from the os-volume_attachments API in the past | |
| 15:00:46 | bauzas | sdague: you create a group thru the API | |
| 15:00:53 | bauzas | and what dansmith said | |
| 15:01:15 | bauzas | no way to remove an instance from a group, long-lived story | |
| 15:01:43 | mriedem | there have been specs for that though, adding and removing members | |
| 15:01:43 | Kevin_Zheng | mriedem: Hmm OK，what I dicovered is | |
| 15:01:46 | mriedem | i remember those from kilo | |
| 15:01:56 | bauzas | yup | |
| 15:02:02 | bauzas | and I can name the person :) | |
| 15:02:08 | bauzas | who is now a Watcher core :) | |