| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-22 | |||
| 00:22:48 | sean-k-mooney | and in nova case i know our mocking is broken on the lower bound as we are actully trying to connect to rabbitmq in some of the test and its not mocked | |
| 00:24:35 | sean-k-mooney | if i bumped the version of oslo.messaging we have in lower bound our tests would pass with the fix to the tox.ini but i dont think we actully need the newer version is just broken tests | |
| 00:25:41 | gmann | it seems few deps are taken from lower-constraints.txt like hacking | |
| 00:25:46 | sean-k-mooney | smcginnis: anyway this is either a change in pips behavior or this was always broken unfortunetly | |
| 00:26:29 | sean-k-mooney | gmann: i think what is happening is any dep that is not n upper-consrtraitng but is in lower-constratits is being honered | |
| 00:26:46 | sean-k-mooney | but i think ip is takeing the constraint form the first file it finds it in | |
| 00:26:52 | sean-k-mooney | that is jsut a guess | |
| 00:28:59 | gmann | seems so. and that is why we have upper-constraints.txt also in dep list but picking order might have changed on tox/pip side | |
| 00:29:33 | sean-k-mooney | well tox just runs pip so tox is not really invoved | |
| 00:30:27 | sean-k-mooney | but for this env specifcially we should not include upper-constraints.txt or at least if we did it should be after lower | |
| 00:31:00 | sean-k-mooney | looking at the pip change log i dont imediatly see something related to this | |
| 00:32:31 | sean-k-mooney | the -U flag proably isnt helping much either in this case | |
| 00:41:24 | gmann | sean-k-mooney: we might need upper-constraints.txt pkg not in lower-constraints.txt, in case latest pip version of them would create issue. | |
| 00:42:36 | sean-k-mooney | gmann: we may yes but we need to validate the ordering we pass them | |
| 00:42:47 | gmann | yeah ordering is imp | |
| 00:43:36 | sean-k-mooney | the docs for pip dont state anything about orderign so im going to look at the code tomorow | |
| 00:44:18 | sean-k-mooney | that said its alreay tomorow i.e. its after mindnight so i should proable leave this untill after i sleep for selveral hours | |
| 00:54:57 | sean-k-mooney | gmann: ok im actully going now but i think this is the isue. https://github.com/pypa/pip/blob/d4217f0cc30ea2012911be1cb22a74d8cc4a365a/src/pip/_internal/req/req_set.py#L133-L136 | |
| 00:55:33 | sean-k-mooney | the way i processses constratits if a constraint is repeated the first one is used | |
| 00:59:38 | sean-k-mooney | looking at git blame the logic seam to be unchange bar minor change for at least 4 years | |
| 00:59:40 | sean-k-mooney | https://github.com/pypa/pip/blame/0bc7a974fac35ebc0b38beeba4de59017166b1bc/src/pip/_internal/req/req_set.py#L109-L130 | |
| 01:02:35 | gmann | humm, i think added upper-constraints.txt dep in tox before lower-constraints.txt things happened. which means it never worked as expected | |
| 01:16:44 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: WIP: Make os-services policies granular https://review.openstack.org/645427 | |
| 01:29:32 | openstackgerrit | melanie witt proposed openstack/nova master: DNM Re-enable testing of console with TLS in nova-next job https://review.openstack.org/645432 | |
| 01:45:46 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: WIP: Make os-services policies granular https://review.openstack.org/645427 | |
| 02:28:29 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: WIP: Make os-services policies granular https://review.openstack.org/645427 | |
| 02:29:26 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: WIP: Make os-services policies granular https://review.openstack.org/645427 | |
| 02:46:05 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Introduce scope_types in os-services, lock policies https://review.openstack.org/645452 | |
| 02:55:15 | openstackgerrit | Merged openstack/nova master: Require python-ironicclient>=2.7.0 https://review.openstack.org/642863 | |
| 03:21:45 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: WIP:Introduce scope_types in os-services, lock policies https://review.openstack.org/645452 | |
| 03:24:14 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Stop running tempest-multinode-full https://review.openstack.org/645456 | |
| 03:24:49 | mriedem | keeel it ^ | |
| 03:30:19 | openstackgerrit | Boxiang Zhu proposed openstack/nova-specs master: Scheduler filters evaluated even forced host https://review.openstack.org/645458 | |
| 03:46:54 | openstackgerrit | Boxiang Zhu proposed openstack/nova-specs master: Scheduler filters evaluated even forced host https://review.openstack.org/645458 | |
| 03:50:05 | openstackgerrit | OpenStack Release Bot proposed openstack/nova stable/stein: Update .gitreview for stable/stein https://review.openstack.org/645461 | |
| 03:50:10 | openstackgerrit | OpenStack Release Bot proposed openstack/nova stable/stein: Update UPPER_CONSTRAINTS_FILE for stable/stein https://review.openstack.org/645462 | |
| 03:50:16 | openstackgerrit | OpenStack Release Bot proposed openstack/nova master: Update master for stable/stein https://review.openstack.org/645463 | |
| 06:22:40 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove deprecated 'default_flavor' config option https://review.openstack.org/645476 | |
| 06:26:32 | openstackgerrit | OpenStack Proposal Bot proposed openstack/nova master: Imported Translations from Zanata https://review.openstack.org/645478 | |
| 06:33:16 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Add description about sort order in API ref guideline https://review.openstack.org/627282 | |
| 06:33:21 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Add description about sort order in API ref guideline https://review.openstack.org/627282 | |
| 09:27:39 | openstackgerrit | Matthew Booth proposed openstack/nova master: Eventlet monkey patching should be as early as possible https://review.openstack.org/626952 | |
| 09:28:28 | openstackgerrit | Lee Yarwood proposed openstack/nova master: Use migration_status during volume migrating and retyping https://review.openstack.org/637224 | |
| 09:45:07 | openstackgerrit | Boxiang Zhu proposed openstack/nova master: [WIP] Scheduler filters evaluated even forced host https://review.openstack.org/645520 | |
| 09:50:20 | kashyap | mdbooth_: Nice work on that eventlet patch (although I'm not fully qualified to comment on it). | |
| 09:50:23 | kashyap | "I started with what seemed like the simplest and most obvious change, and moved on as I discovered more interactions which broke." | |
| 09:50:26 | kashyap | :D | |
| 09:51:01 | kashyap | (Also kudos for the "commit message mini essay") | |
| 09:51:22 | mdbooth | kashyap: Heh, thanks. The commit message has also grown over time :) | |
| 09:51:42 | kashyap | Yes, I can see. I'm sure you spent many hours fine-tuning it as you discovered things. | |
| 09:53:05 | mdbooth | I feel that enormous comments are an indicator of code smell, though, and nova.monkey_patch is almost all comment. | |
| 09:53:39 | mdbooth | Most code should not require that much explanation. | |
| 09:56:47 | openstackgerrit | Merged openstack/nova master: Cleanup comments around claim_resources method https://review.openstack.org/644576 | |
| 09:56:53 | openstackgerrit | Merged openstack/nova master: Address old TODO in claim_resources_on_destination https://review.openstack.org/644596 | |
| 10:30:45 | lyarwood | mdbooth: re https://review.openstack.org/#/c/637224/ - good news, I can't reproduce the cinder weirdness locally after rebasing and recloning everything this morning. Just pushed https://review.openstack.org/#/c/637527/ again, hopefully this also passes in the check gate now. | |
| 10:30:54 | lyarwood | s/gate/queue/g | |
| 10:33:59 | kashyap | mdbooth: Yeah, was about to remark on that, too. THe large commentary section. | |
| 11:53:14 | openstackgerrit | Matthew Booth proposed openstack/nova master: Fix incomplete instance data returned after build failure https://review.openstack.org/645546 | |
| 12:55:06 | kaisers | mdbooth: Hi! If possible, could you revisit the driver bugfix at https://review.openstack.org/#/c/554195/ ? Needs a second +2 and you already looked into it some time ago. :) | |
| 12:55:45 | mdbooth | kaisers: I don't have that mojo, but I can take a look. | |
| 12:56:20 | kaisers | mdbooth: still fine, thanks | |
| 13:04:55 | openstackgerrit | Merged openstack/nova master: Imported Translations from Zanata https://review.openstack.org/645478 | |
| 13:11:46 | openstackgerrit | Surya Seetharaman proposed openstack/nova-specs master: Support adding the reason behind a server lock https://review.openstack.org/638629 | |
| 13:13:18 | tssurya | cdent, edleaf, mriedem: thanks for the spec review ^ I have updated it with respect to your comments. | |
| 13:13:35 | cdent | tssurya: great, thank you. will look again soon. | |
| 13:13:44 | tssurya | edleafe* | |
| 13:13:58 | edleafe | tssurya: :) | |
| 13:15:22 | tssurya | mriedem: I have a question regarding the notification change: https://review.openstack.org/#/c/638629/1/specs/train/approved/add-locked-reason.rst@168 not particularly what/why it should change | |
| 13:17:31 | gibi | tssurya: I think it is not mandatory to add the new fields to the notification payload but it would be nice to add | |
| 13:17:44 | gibi | tssurya: if we are not adding now, then we can add it later | |
| 13:18:49 | tssurya | gibi: ah ok, yea I can add them in this spec itself, np, I wasn't sure of the workflow since I haven't dealt with notification changes before | |
| 13:19:04 | tssurya | so that means I need to add the fields into the InstancePayload | |
| 13:19:14 | tssurya | and then send that via the notifications event right ? | |
| 13:20:07 | gibi | tssurya: yes, you extend the IntancePayload and with the new fields and make sure that the value of the new fields are populated from the instance | |
| 13:20:17 | tssurya | https://github.com/openstack/nova/blob/b33fa1c054ba4b7d4e789aa51250ad5c8325da2d/nova/compute/utils.py#L447 somewhere there ? | |
| 13:20:52 | tssurya | oh ok | |
| 13:21:24 | gibi | tssurya: there is a schema def in the InstancePayload ovo that defined how to copy data from the Instance ovo so if you fill the schema then the copy is automatic | |
| 13:21:32 | gibi | https://github.com/openstack/nova/blob/master/nova/notifications/objects/instance.py#L59 | |
| 13:22:22 | tssurya | gibi: ah okay that's great | |
| 13:22:36 | tssurya | sure then I will add this change too to the spec | |
| 13:22:40 | tssurya | thanks gibi! :) | |
| 13:22:50 | gibi | tssurya: here is a patch from the past that extended the InstancePayload https://github.com/openstack/nova/commit/2bca6431e69bf2c6e657736b7fe11f5a2fbb9433 | |
| 13:26:40 | tssurya | gibi: nice, I can follow that workflow :) | |
| 13:27:04 | gibi | tssurya: and you can invite me to review the patch when it is up. :) | |
| 13:27:10 | openstackgerrit | Mohammed Naser proposed openstack/nova master: bdm: store empty object as connection_info by default https://review.openstack.org/645352 | |
| 13:27:22 | tssurya | gibi: absolutely! :D thank you | |
| 13:27:26 | mnaser | i added a note to this bug. it's been around for a few cycles ^ | |
| 13:27:26 | gibi | np | |
| 13:32:17 | smcginnis | sean-k-mooney: I see now what you were saying with the lower-constraints issue yesterday. | |
| 13:32:22 | smcginnis | sean-k-mooney: Cinder has this: https://opendev.org/openstack/cinder/src/branch/master/tox.ini#L15-L18 | |
| 13:32:40 | smcginnis | sean-k-mooney: That's what you were saying the nova tox.ini needed to be changed to, right? | |
| 13:34:11 | cdent | smcginnis , sean-k-mooney I have a patch pending to fix lower constraints in nova for months and months https://review.openstack.org/#/c/622972/ | |
| 13:34:42 | cdent | it's so old now it is probably out of date | |
| 13:35:06 | smcginnis | Well wouldya look at that. | |
| 13:35:47 | smcginnis | I do think we should probably have the deps set separately than install_command though. | |
| 13:35:58 | smcginnis | Since that's the variable between these targets. | |
| 13:36:09 | smcginnis | But two ways to do the same thing I suppose. | |
| 13:39:24 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Update contributor guide for Train https://review.openstack.org/645581 | |
| 13:47:06 | openstackgerrit | Merged openstack/nova master: Trivial: remove unused var from policies.base.py https://review.openstack.org/645380 | |
| 13:58:22 | fried_rice | o/ | |