Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-06-13
15:58:45 cfriesen mriedem: andreykurilin: for some reason I can't do pdb in novaclient/v2/shell.py, it just hangs. wonder if that's related to the issue
15:59:31 andreykurilin it is strange, because I fetched your patch, applied changes in setUp method and everything works for me(at least test_default_endpoint_type passes)
15:59:55 cfriesen andreykurilin:did you change it from "flavor-list" to "list"?
16:00:12 andreykurilin ok, let me try
16:00:18 cfriesen in test_default_endpoint_type9)
16:02:10 mriedem alex_xu: sdague: remember how we wanted to do the os-services and os-hypervisors id->uuid API changes in the same microversion, plus a bunch of other cleanups for weird PUT and GET APIs?
16:02:26 mriedem this one https://specs.openstack.org/openstack/nova-specs/specs/pike/approved/service-hyper-uuid-in-api.html
16:02:28 openstackgerrit Sean Dague proposed openstack/python-novaclient master: Fix setting 'global_request_id' in SessionClient https://review.openstack.org/473224
16:03:03 mriedem turns out that might be a monster change if we're doing it on the same microversion, like the os-services one is pushing 1000 LOC
16:03:10 andreykurilin cfriesen: `self.mock_client.return_value.api_version = api_versions.APIVersion("2.1")`
16:03:16 andreykurilin you need to setup api_version like this
16:04:07 andreykurilin cfriesen: because self.mock_client is just a mock of novaclient.client.Client method and CLI obtains api_version property from the object which that novaclient.client.Client returns
16:04:22 sdague mriedem: because of the tests and friends?
16:04:31 mriedem sdague: and docs yeah
16:04:32 mriedem plus,
16:04:38 mriedem os-hypervisors is going to require changes for paging across cells,
16:04:43 mriedem which i didn't originally anticipate in the spec
16:04:46 sdague ok
16:05:02 sdague is there any way to structure that code for land better?
16:05:29 mriedem i've tried to split out some of it by layers, e.g. https://review.openstack.org/#/c/461532/
16:05:37 mriedem is the compute API stuff, before the REST API controller pieces
16:05:54 mriedem can't really do that once i get to https://review.openstack.org/#/c/464280/ though
16:05:58 sfinucan sdague: Yeah, go for it
16:05:59 mriedem granted that one is mostly tests and docs
16:06:14 sfinucan (pulling them together)
16:06:17 sdague honestly, I have wondered if we should stack our API changes a little differently so the functional is in one patch, samples tests in next patch, api-ref in 3rd? Then just -2 the bottom and do a flush
16:06:37 sdague because I do agree they get complicated to mentally context switch in the reviews
16:06:38 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Handle uuids in os-hypervisors API https://review.openstack.org/471899
16:06:39 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Use uuid for id in Services API https://review.openstack.org/464280
16:06:48 mriedem ^ is the latest before i have the api-ref working
16:06:54 mriedem per my os_api_ref problem this morning
16:07:12 mriedem my plan here was to get reviews on each separately, -2 on os-services until +2 on os-hypervisors, then squash them
16:07:37 mriedem there is going to be db api/object layer paging changes for the hypervisors one that will be separate
16:07:40 mriedem but things still get pretty big
16:07:46 cfriesen andreykurilin: you're my hero. :)
16:07:59 mriedem https://review.openstack.org/#/c/464280/11/nova/api/openstack/compute/services.py is the main functional piece in the os-services change, and isn't too bad
16:08:18 mriedem i chose not to try and re-using the weird little private methods for the service action apis
16:08:22 mriedem like _enable and _disable
16:08:36 mriedem so it's a single update() method for the new microversion that handles all of the things in the request
16:12:08 openstackgerrit Sean Dague proposed openstack/python-novaclient master: Fix setting 'global_request_id' in SessionClient https://review.openstack.org/473224
16:12:16 sdague sfinucan: it can actualy be a little smaller
16:13:30 sfinucan sdague: And even smaller? REQ_ID_HEADER looks unused
16:14:41 sdague sfinucan: oh, it might be now
16:14:49 bauzas mriedem: I did remember how cutting the relationship between services and hypervisors was a big thing, so I trust you when you say it's becoming a monster change
16:14:52 sdague I can swing around in a sec
16:15:08 sfinucan no rush
16:15:35 bauzas touching services and hypervisor seems like the most impactful things to do, because we have a shit ton of tests asserting things, plus a lot of logic around those
16:15:46 openstackgerrit Sean Dague proposed openstack/python-novaclient master: Fix setting 'global_request_id' in SessionClient https://review.openstack.org/473224
16:15:50 mriedem it's definitely not fun
16:15:57 bauzas mriedem: any series I can start to review, so I could help ?
16:16:03 mriedem the os-services thing is pretty straight-forward though, after working on it for a couple of days
16:16:03 bauzas don't be afraid of WIPs
16:16:16 mriedem bauzas: https://review.openstack.org/#/c/464280/11 and below
16:16:27 bauzas mriedem: okay, I'll review those
16:16:32 mriedem bauzas: ^ is really basically done, except one todo in the code and i need to get the api-ref working
16:16:56 mriedem sdague: i wonder if my comments in the parameters.yaml is what's making os_api_ref puke?
16:17:10 bauzas mriedem: I can see the code
16:17:39 mriedem oh i guess not, there are already comments in there
16:18:50 sdague mriedem: you have the patch?
16:18:57 mriedem https://review.openstack.org/#/c/464280/
16:21:25 sdague ok, let me poke
16:25:55 sdague mriedem: ok, it's a warning that's trying to trigger
16:26:31 sdague oh, i see what's going on, let me figure out the right fix
16:33:12 sdague oh, it looks like sphinx changed a call interface
16:34:05 mriedem ok but this would be the warning https://github.com/openstack/os-api-ref/blob/master/os_api_ref/__init__.py#L349
16:34:15 mriedem i'm missing some path parameter
16:37:27 openstackgerrit Alex Szarka proposed openstack/nova master: Refactor create_delete_server_with_instance_update https://review.openstack.org/466296
16:38:38 openstackgerrit Chris Friesen proposed openstack/python-novaclient master: 2.47: Show flavor info in server details https://review.openstack.org/435141
16:38:38 openstackgerrit Chris Friesen proposed openstack/python-novaclient master: Clean up ShellTest unit tests https://review.openstack.org/473910
16:38:55 cfriesen mriedem: andreykurilin: ^
16:40:05 mriedem sdague: yeah if i restrict Sphinx<1.6.1 i get this
16:40:05 mriedem /home/user/git/nova/api-ref/source/api-ref/source/os-services.inc:250 .rst:: WARNING: No path parameter ``service_id`` found in rest_parameter stanza.
16:40:21 sdague yeh
16:40:28 openstackgerrit Balazs Gibizer proposed openstack/nova master: Transform missing delete notifications https://review.openstack.org/410297
16:40:44 sdague so they seem to have changed the warn parameters on env
16:43:23 mriedem i'll open a bug quick to track it against os-api-ref
16:43:31 cfriesen mriedem: for https://review.openstack.org/#/c/465787/ we have some indication that it's not properly handling neutron floatingip-associate events
16:44:10 openstackgerrit Merged openstack/nova master: Fix the race condition with novnc https://review.openstack.org/470798
16:53:40 openstackgerrit Merged openstack/nova master: Add missing query filter params for GET /os-services API https://review.openstack.org/473708
16:55:51 mriedem dansmith: about that service not found thing last night, where it's not a compute service to do things like enable/disable,
16:55:59 mriedem we talked about returning a more useful error message in that case,
16:56:11 mriedem however, it's a chicken and egg problem,
16:57:04 mriedem well, maybe not - if it's not a compute service it won't have a host mapping and we'll get HostMappingNotFound,
16:57:18 mriedem if it is a compute service and really just doesn't exist, we'll get ServiceNotFound
16:59:12 mriedem anyway, now i'm having a hard time recalling what we talked about doing here
16:59:50 dansmith not 404ing if they try to disable a scheduler for example
17:00:13 mriedem oh right, so it is kind of a chicken and egg then
17:00:18 mriedem we don't know it's a scheduler or not
17:00:25 mriedem if there is no host mapping, that could be b/c it's not mapped yet
17:00:33 mriedem or because it's not a compute service
17:01:55 mriedem anyway, i think i'm going to write something up to the ML about this just to make people aware and see if anyone had any weird uses for disabling non-compute services,
17:02:01 mriedem to help answer what i should do in the new api
17:02:21 dansmith if we don't find a mapping, we can lookin cell0 for the service, and if we find, it we can assume it's not an unmapped compute, but a non-compute control service
17:02:37 dansmith there is no use for disabling a non-compute service,
17:02:44 dansmith all it does is set disabled=True, and that's only checked in the scheduler
17:02:53 dansmith I went through this with someone internal a few months ago
17:03:14 mriedem i know, but you never know what people are doing - which is why i just want to ask or let people be aware of this,
17:03:25 mriedem i think we still go forward with restricting updating to only compute services
17:03:57 mriedem i.e. i could totally see someone using service.update notifications to trigger some backend system to kill their nova-scheduler or something, so they can control things via the api
17:05:11 dansmith and your point is you just want to alert those people that we're killing that? .... RIGHT?

Earlier   Later