| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-18 | |||
| 02:22:24 | gmann | oomichi: yea | |
| 02:28:28 | oomichi | takashin: can you take a look at https://review.openstack.org/#/c/457167 ? I am not sure the error status code 503 also is necessary for evacuate API | |
| 02:29:14 | takashin | oomichi: Thank you for your review. | |
| 02:30:08 | takashin | oomichi: I don't know 503 is necessary or not, too. | |
| 02:33:24 | oomichi | takashin: I did some investigation now and actually APIs which could return 503 are fping and tenant-network only | |
| 02:33:45 | oomichi | takashin: the corresponding api-ref contains it on these APIs. | |
| 02:34:05 | oomichi | takashin: the other api-ref which includes this case should be invalid I think now | |
| 02:34:26 | takashin | oomihi: Thanks. I will remove 503 in evacuate API reference. | |
| 02:34:35 | oomichi | takashin: cool, thanks | |
| 02:48:25 | oomichi | gmann: about https://review.openstack.org/#/c/456430 I did think about it before, but I don't have strong opinion about that | |
| 02:48:57 | oomichi | gmann: the enum list clearly says the type is string even if not having 'type' | |
| 02:49:28 | oomichi | gmann: it is also an option to remove 'type' from such definitions instead | |
| 02:49:41 | oomichi | for consistency | |
| 02:50:54 | gmann | oomichi: actually, enum values can be accessed with index so it was confusing to me. but yes json schema defien enum always a string i think | |
| 02:53:34 | oomichi | gmann: it also would be fine to add type to all schema, but current patch seems necessary to be updated anyways | |
| 02:55:04 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: api-ref: Fix response code and parameters in evacuate https://review.openstack.org/457167 | |
| 02:55:21 | gmann | oomichi: did not get, you mean to updated with takashin comments ? | |
| 02:55:34 | oomichi | gmann: yep | |
| 02:56:09 | gmann | oomichi: yea i can incorporate those separately if takashin is ok :) | |
| 02:56:21 | gmann | takashin: ^^ what you say? | |
| 03:02:00 | takashin | gmann: You can do it separately. But it is small changes. So it is good for reviewers to do it together. | |
| 03:09:29 | gmann | takashin: sure, ll do | |
| 03:30:02 | oomichi | takashin: one question about https://review.openstack.org/#/c/414926/39/nova/tests/functional/test_servers.py | |
| 03:30:27 | oomichi | takashin: why do we need to specify a certain image_uuid for these test cases? | |
| 03:31:15 | takashin | oomichi: Just a moment, please. | |
| 03:31:22 | oomichi | takashin: New tests seem to require the same host only, so I could not understand why we need to specify image_uuid | |
| 03:54:22 | openstackgerrit | Huan Xie proposed openstack/nova master: WIP: XenAPI use os-xenapi V2 in nova https://review.openstack.org/453493 | |
| 03:55:20 | takashin | oomichi: Image API was deprecated in API version 2.36. | |
| 03:55:50 | takashin | oomichi: So an error is raised in https://github.com/openstack/nova/blob/5a556a720f5a7394cab4c84fa6202976c6190b23/nova/tests/functional/integrated_helpers.py#L141 . | |
| 03:56:51 | oomichi | takashin: does that mean 2.36+ functional tests should specify image_uuid, right? | |
| 03:57:02 | takashin | oomichi; And I modified _build_minimal_create_server_request method in _IntegratedTestBase, | |
| 03:57:55 | takashin | oomichi: Yes. (in my modification.) | |
| 03:58:37 | oomichi | takashin: I see, thanks. Then it would be better to have NOTE on https://review.openstack.org/#/c/414926/39/nova/tests/functional/integrated_helpers.py | |
| 03:58:50 | oomichi | to explain the above your comment | |
| 03:59:11 | takashin | oomichi: Thank you for your review. I will add a note. | |
| 03:59:21 | oomichi | takashin: thanks again :) | |
| 05:14:46 | openstackgerrit | Huan Xie proposed openstack/nova master: XenAPI: Create linux bridge in dest host during live migration https://review.openstack.org/451657 | |
| 05:37:18 | openstackgerrit | Huan Xie proposed openstack/nova master: WIP: XenAPI use os-xenapi V2 in nova https://review.openstack.org/453493 | |
| 05:56:33 | openstackgerrit | Huan Xie proposed openstack/nova master: WIP: XenAPI use os-xenapi V2 in nova https://review.openstack.org/453493 | |
| 06:24:52 | openstackgerrit | Huan Xie proposed openstack/nova master: WIP: XenAPI use os-xenapi V2 in nova https://review.openstack.org/453493 | |
| 06:26:49 | openstackgerrit | Huan Xie proposed openstack/nova master: WIP: XenAPI use os-xenapi V2 in nova https://review.openstack.org/453493 | |
| 07:28:36 | openstackgerrit | Alex Xu proposed openstack/nova master: Deprecate Multinic, floatingip action and os-virtual-interface API https://review.openstack.org/457181 | |
| 07:35:59 | openstackgerrit | Zhenyu Zheng proposed openstack/nova master: Support tag instances when boot https://review.openstack.org/394321 | |
| 07:39:24 | openstackgerrit | shaofeng cheng proposed openstack/nova master: Update nova version newton to ocata in upgrade.rst https://review.openstack.org/457197 | |
| 07:39:55 | openstackgerrit | Gábor Antal proposed openstack/nova master: Transform instance.volume_attach.error notification https://review.openstack.org/455801 | |
| 07:43:58 | gcb | alex_xu: please help review https://review.openstack.org/457188 , that fixes http://logs.openstack.org/periodic/periodic-nova-py35-with-oslo-master/74800d1/testr_results.html.gz | |
| 07:45:03 | gcb | alex_xu: this blocks we release oslo.config 4.0, hope we can fix it in Nova side before we release new version of oslo.config | |
| 08:27:05 | openstackgerrit | Lei Zhang proposed openstack/nova master: Add sync traits command for placement https://review.openstack.org/450125 | |
| 08:27:46 | openstackgerrit | LiChaoLei proposed openstack/nova master: Fix bug of delete image_base_image_ref after rebuild instance https://review.openstack.org/457514 | |
| 08:50:13 | openstackgerrit | Gábor Antal proposed openstack/nova master: [WIP] Transform instance.live_migration_rollback notification https://review.openstack.org/402124 | |
| 09:02:40 | openstackgerrit | Huan Xie proposed openstack/nova master: WIP: XenAPI use os-xenapi V2 in nova https://review.openstack.org/453493 | |
| 09:09:42 | openstackgerrit | Roman Podoliaka proposed openstack/osc-placement master: tests: add a hook for functional testing in the gate https://review.openstack.org/452122 | |
| 09:09:43 | openstackgerrit | Roman Podoliaka proposed openstack/osc-placement master: CLI for inventories https://review.openstack.org/457533 | |
| 09:09:43 | openstackgerrit | Roman Podoliaka proposed openstack/osc-placement master: CLI for resource providers https://review.openstack.org/457532 | |
| 09:09:44 | openstackgerrit | Roman Podoliaka proposed openstack/osc-placement master: CLI for usages https://review.openstack.org/457535 | |
| 09:09:44 | openstackgerrit | Roman Podoliaka proposed openstack/osc-placement master: CLI for allocations https://review.openstack.org/457534 | |
| 09:13:13 | openstackgerrit | Zhenyu Zheng proposed openstack/nova master: Support tag instances when boot https://review.openstack.org/394321 | |
| 09:23:37 | alex_xu | gcb: done | |
| 09:43:26 | openstackgerrit | Alex Xu proposed openstack/nova master: Deprecate Multinic, floatingip action and os-virtual-interface API https://review.openstack.org/457181 | |
| 10:05:46 | openstackgerrit | falseuser proposed openstack/nova master: [BugFix] Check the instance task status to release the memory quota for vram https://review.openstack.org/456050 | |
| 10:15:17 | johnthetubaguy | cdent: am I right to worry about this one? https://review.openstack.org/#/c/448791/7 | |
| 10:15:37 | cdent | johnthetubaguy: reading | |
| 10:17:07 | gcb | alex_xu: thanks | |
| 10:18:30 | cdent | johnthetubaguy: your comments are mostly reasonable (and you did find a bug), but I'm not sure you should "worry"? | |
| 10:18:44 | johnthetubaguy | cdent: heh, I am just a worrier | |
| 10:19:08 | johnthetubaguy | cdent: s/1.5/1.6/ I guess? | |
| 10:19:30 | cdent | the pattern for how to deal with microversions in the gabbi tests has yet to emerge, in that particular file it is "put diferences near to one another so we can compare" | |
| 10:19:42 | cdent | but it might be better to do what you suggest "group the new version" | |
| 10:19:42 | johnthetubaguy | cdent: I was curious about the gabby stuff, do we really want to modify those old tests to request a different version? | |
| 10:19:56 | cdent | the 1.5, 1.6 thing is the bug | |
| 10:20:20 | johnthetubaguy | yeah, but those tests, we should probably best both request no version, and request 1.6 I guess? | |
| 10:20:28 | cdent | the use of a version is to be explicit in the test so that it is clear that we are instentionally saying "I want a version that has a version I know" rather than "I want the default version" | |
| 10:20:44 | cdent | the intention isn't to test that microversions work | |
| 10:20:48 | cdent | that's tested elsewhere | |
| 10:21:03 | cdent | the intention is to test that the behavior at known microversions is what we expect, and to be explicit about it | |
| 10:21:07 | cdent | however | |
| 10:21:31 | cdent | given that you found a bug in reading the code that the tests did not catch, there's probably an adjustment that needs to be made | |
| 10:22:28 | johnthetubaguy | I know before in nova-api we said, test bottom, test top, test change points | |
| 10:23:14 | johnthetubaguy | we missed some of the test change points in the past though, and end up with a discontinuity in bits of the API being supported, i.e. we missed the "change" point that needed testing | |
| 10:23:14 | cdent | that might be a good idea, but we haven't established that yet | |
| 10:23:28 | johnthetubaguy | not in placement, I think that was assumed in nova-api | |
| 10:23:46 | cdent | I'll look at making it more betterer | |
| 10:24:25 | johnthetubaguy | cdent: I guess I just like seeing tests not changed when I don't want the behaviour to change, if that makes sense | |
| 10:24:38 | cdent | yeah, that makes sense | |
| 10:24:49 | johnthetubaguy | I just don't trust my eyes to review that stuff properly | |
| 10:25:31 | cdent | I think adding instead of changing is probably a good idea. It is a bit harder to do cleanly in the gabbi stuff because we're in a single session there and I really want to avoid this situation where we create giant gabbi files. Might need to do some of it in separate files. | |
| 10:25:43 | johnthetubaguy | testing all those negative tests for top a middle makes no sense to me either, so clearly a middle ground there | |
| 10:25:51 | johnthetubaguy | yeah, thats true | |
| 10:27:13 | johnthetubaguy | I was thinking you just repeat one negative test with version 1.6, to confirm it works, a test to confirm the API is removed in 1.7, just leave the old tests as they are, and test the new 1.7 in isolation, was what I was thinking? | |
| 10:27:22 | johnthetubaguy | not sure if that fits with what you were thinking? | |
| 10:28:59 | cdent | it sounds reasonable, I'll have to see what comes out when I start playing with it properly. | |
| 10:29:12 | johnthetubaguy | yeah | |
| 10:31:15 | cdent | but yeah, thank you, is very good input | |
| 10:31:38 | johnthetubaguy | no worries, happy to help | |
| 10:37:57 | cdent | johnthetubaguy: on the RT, the fallback was something that matt said ought to happen, to be extra nice. I'm not sure about adding the warning. Is that the usual behavior? Seems noisy to me. | |
| 10:38:19 | johnthetubaguy | cdent: we sometimes only warn the first time, but that does overcomplicate things | |
| 10:38:38 | johnthetubaguy | cdent: maybe debug is best, I am just thinking about scratching my head debugging things in the future really | |
| 11:15:04 | cdent | johnthetubaguy: ah, part of the reason for those explicit microversions is because the tests in tha file are using a default of 'latest' | |
| 11:15:22 | johnthetubaguy | cdent: ah... I missed that | |
| 11:16:08 | cdent | I still think there are some good cleanups that can be made based on your idea though. | |