| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-06 | |||
| 18:52:46 | jaypipes | regardless... | |
| 18:52:50 | mdbooth | https://review.openstack.org/#/c/242603/25/nova/objects/block_device.py | |
| 18:54:19 | mdbooth | jaypipes: Thing is, if you overthink the building blocks, it means you don't need to overthink everything else. Not so much, anyway. | |
| 18:54:44 | mdbooth | The caller doesn't need to be concerned with all these weird edge cases. It just works. | |
| 19:00:26 | openstackgerrit | Merged openstack/nova master: Fix doubling allocations on rebuild https://review.openstack.org/521662 | |
| 19:01:51 | zzzeek | mdbooth / jaypipes the retry decorartor is wanting, IMO | |
| 19:02:05 | zzzeek | there was originally some idea to integrate one into enginefacade itself so that it knows how to nest | |
| 19:02:40 | mdbooth | zzzeek: Could you use a savepoint for this, btw? | |
| 19:03:10 | zzzeek | mdbooth: not if you're looking to respond to other transactions also making changes b.c. isolation may prevent you from seeing those changes | |
| 19:03:40 | zzzeek | for an UPDATE i think repeatable read would mean you can't see another transactions updates | |
| 19:05:31 | mdbooth | zzzeek: Anyway, my approach was to use an independent transaction context explicitly. I figured this would always be safe. | |
| 19:06:02 | mdbooth | Then I was convinced it wasn't worth writing slight weird code for an extremely obtuse edge case where failure was safe. | |
| 19:06:03 | zzzeek | mdbooth: i think you are correct assuming that record is not part of the external tranasction. pre-enginefacade nova was all independent transactions. | |
| 19:06:37 | mdbooth | But then I flipped again because I figured this is a read transaction which only needs to write in an edge case (migrating a legacy record). | |
| 19:07:10 | mdbooth | So having an independent transaction is useful to avoid forcing the enclosing transaction and all its potential parents to become writers. | |
| 19:08:06 | mdbooth | And that's where I am now. | |
| 19:10:38 | zzzeek | mdbooth: i think leave it and goad me into working up a better retry situation for enginefacade | |
| 19:11:08 | mdbooth | zzzeek: So all of the above is in the context of code we'd want to delete next cycle anyway, as it's only for a migration :) | |
| 19:11:48 | mdbooth | Well cycle +1, because the online migration runs it too. | |
| 19:11:48 | zzzeek | mdbooth: nova's magical migrations...sure | |
| 19:11:52 | zzzeek | mdbooth: sure | |
| 19:12:00 | zzzeek | mdbooth: this is not the most concerning thing to me :) | |
| 19:13:10 | mdbooth | I think it's fine, tbh. I also doubt that many of nova's uses of the retry wrapper are a problem today, because we don't nest many transactions. | |
| 19:13:35 | mdbooth | It would be a bigger problem if we made wider use of the enginefacade | |
| 19:14:29 | mdbooth | jaypipes: Thanks again | |
| 19:22:30 | openstackgerrit | Chris Dent proposed openstack/nova master: Optional separate database for placement API https://review.openstack.org/362766 | |
| 19:33:20 | cdent | can someone besides mriedem put some eyes on https://review.openstack.org/#/c/523403/ is an auth config error that would be useful to backport. dansmith, jaypipes ? | |
| 19:33:55 | efried | cdent Patches are passing with your grenade fix - thanks for that. | |
| 19:34:06 | cdent | huzzah | |
| 19:34:30 | mriedem | cdent: i think melwitt enjoys auth config bug fixes | |
| 19:34:59 | melwitt | I totally do | |
| 19:35:02 | cdent | ah yes melwitt would be a fine choice | |
| 19:35:28 | cdent | apologies had not hear you around recently | |
| 19:35:35 | cdent | whereas dansmith and jaypipes … | |
| 19:35:57 | efried | cdent Is the kwarg really called oslo_config_config? | |
| 19:36:19 | cdent | yeah, isn’t that awesome, there are a few different olso_config_* | |
| 19:36:54 | efried | whee | |
| 19:37:42 | cdent | I think it is to try to be explicit about the fact that it can also accept keyword arguments which are configuration keys and values | |
| 19:38:05 | cdent | a sort of namespacing | |
| 19:41:27 | sean-k-mooney2 | cdent: we were going to do something similar with networking-ovs-dpdk devstack setting then reallised that would haver ment setting OVS_DPDK_OVS_REPO AND OVS_DPDK_DPDK_REPO so decided not to prepend everyting with OVS_DPDK | |
| 19:42:08 | cdent | oh my | |
| 19:42:52 | sean-k-mooney2 | we still have pretty verbose setting without that https://github.com/openstack/networking-ovs-dpdk/blob/master/devstack/settings | |
| 19:42:53 | efried | cdent Left a comment on that patch. Is there a reason we would need to keep that arg? | |
| 19:43:49 | cdent | efried: because we don’t know who might have use the deploy() method in the past | |
| 19:44:01 | cdent | that’s how you build a custom wsgi application, if you’re so inclined | |
| 19:44:07 | efried | cdent Is it somehow public? | |
| 19:44:24 | cdent | it is public by virtue of being python | |
| 19:44:47 | efried | What I mean is, is it used by some known external entity that we care about? | |
| 19:44:52 | cdent | and in the early days of placement was the entry point to the wsgi application stack until sean added the stuff in wsgi.py | |
| 19:44:57 | cdent | the point is that we don’t know | |
| 19:45:09 | efried | But we care? | |
| 19:45:13 | cdent | yes, of course | |
| 19:45:18 | efried | Oh, not of course. | |
| 19:45:33 | cdent | the whole point of a wsgi application is to be able to integrate it in whatever way you want | |
| 19:45:36 | efried | Nova changes "public APIs" like ComputeDriver all the time without worrying about external consumption. | |
| 19:45:47 | sean-k-mooney2 | was it ever the entrypoint in a version of the placement api that shipped in a release | |
| 19:46:07 | efried | And when I've asked about that before, it was basically explained away to "we don't care". | |
| 19:46:15 | efried | So in this case, why do we care? | |
| 19:46:16 | cdent | sean-k-mooney2: a) I _think_ so but am not certain, b) it has always been my intent that deploy be available as a public interface | |
| 19:46:28 | cdent | because placement is not nova? | |
| 19:46:46 | cdent | it has a _much_ different attitude towards building the wsgi application than the nova-api ever did | |
| 19:47:13 | sean-k-mooney2 | cdent: well if that was the intent then we should treat it as public and document that fact. | |
| 19:48:13 | cdent | I think I probably made the mistake of assuming that was somehow obvious, by virtue of its position in the wsgi stack, but I think I may be one of the few people around here who think of the wsgi stack in that fashion | |
| 19:48:23 | cdent | we can certainly document it as such | |
| 19:49:09 | cdent | efried: would your preference be that I add that documentation to that patch now, or is it cool to worry about it later? | |
| 19:49:32 | efried | Later is fine. | |
| 19:50:50 | efried | cdent Changed sides. | |
| 19:51:25 | cdent | thanks, I’ll do the documenting likely tomorrow, I think I’m out of code mode for today | |
| 19:51:36 | cdent | also, all my promises to myself to not work late tonight seem to be for naught | |
| 19:52:21 | sean-k-mooney2 | cdent: your based in the uk right? | |
| 19:52:24 | cdent | yeah | |
| 19:52:33 | cdent | and I often work this late, but last night I worked past midnight | |
| 19:52:40 | cdent | so was going to give myself a break | |
| 19:52:50 | cdent | but everybody here was just too interesting today | |
| 19:53:14 | sean-k-mooney2 | i have a daily reminder to head home at 8 so im normally here till around now too. | |
| 19:56:00 | cdent | as it is 5 to 8 I think I’ll go, have a good night every one | |
| 20:09:16 | melwitt | mriedem: in case you didn't see, I had posted a follow up for the old quotas code removal that we talked about some days ago https://review.openstack.org/#/c/524234 | |
| 20:12:29 | melwitt | although I think I just realized an issue with it and commented | |
| 20:12:43 | mriedem | lgtm | |
| 20:13:19 | mriedem | oh right | |
| 20:13:30 | mriedem | gawd | |
| 20:14:21 | melwitt | yeah. guh | |
| 20:15:33 | mriedem | https://review.openstack.org/#/c/400384/ | |
| 20:15:58 | melwitt | I need to add some kind of equivalent test to the API tests somewhere. the api samples test cover the presence of the 'reserved' key but not the expected value if no-op quotas | |
| 20:16:13 | mriedem | well, | |
| 20:16:27 | mriedem | easiest is probably an api samples test with the noop quota driver and it's own samples | |
| 20:23:56 | melwitt | lemme see if I can do that | |
| 20:25:33 | openstackgerrit | Hongbin Lu proposed openstack/nova master: Add support for listing hosts in cellv2 https://review.openstack.org/524755 | |
| 20:41:21 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add new style volume attachment support to block_device.py https://review.openstack.org/525787 | |
| 20:41:22 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Implement new attach Cinder flow https://review.openstack.org/330285 | |
| 20:41:22 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: libvirt: Allow multiple volume attachments https://review.openstack.org/267587 | |
| 20:41:23 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Allow multi-attach in compute api https://review.openstack.org/271047 | |
| 20:41:23 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Avoid stashed connector lookup for new style detach https://review.openstack.org/526182 | |
| 20:46:51 | efried | jaypipes I seem to have flubbed https://review.openstack.org/#/c/521605/ which was supposed to be two separate commits (one to add agg stuff to ProviderTree, one to add traits stuff). | |
| 20:47:05 | efried | jaypipes I was about to split it up, but thought I should ask you whether it's worth bothering | |
| 20:47:53 | efried | jaypipes i.e. should I just do both in one change set? | |
| 21:03:46 | mriedem | ildikov: ok, final comments in https://review.openstack.org/#/c/330285/173 | |
| 21:03:54 | mriedem | ildikov: easy things to address and then i'm ready to go on that | |
| 21:04:03 | ildikov | mriedem: ok | |
| 21:04:05 | mriedem | we'll have to decide if we hold it for the shared_targets microversion in cinder | |