| *** dhellmann has joined #openstack-requirements | 00:07 | |
| *** hongbin has joined #openstack-requirements | 02:13 | |
| *** fungi has quit IRC | 03:06 | |
| *** fungi has joined #openstack-requirements | 03:09 | |
| *** fungi has quit IRC | 03:10 | |
| *** fungi has joined #openstack-requirements | 03:40 | |
| openstackgerrit | Arun S A G proposed openstack/requirements stable/ocata: RHEL7 has updated libvirt to 4.5.x https://review.openstack.org/616405 | 03:41 |
|---|---|---|
| *** fungi has quit IRC | 03:41 | |
| *** fungi has joined #openstack-requirements | 03:45 | |
| openstackgerrit | Merged openstack/requirements master: Exclude oslo.cache 1.31.1 https://review.openstack.org/615935 | 04:57 |
| *** hongbin has quit IRC | 06:02 | |
| openstackgerrit | OpenStack Proposal Bot proposed openstack/requirements master: Updated from generate-constraints https://review.openstack.org/616423 | 06:19 |
| prometheanfire | I feel like train is by far the strongest | 07:25 |
| *** bnemec has quit IRC | 08:00 | |
| openstackgerrit | Dirk Mueller proposed openstack/requirements master: update constraint for oslo.service to new release 1.32.1 https://review.openstack.org/615676 | 08:09 |
| *** bnemec has joined #openstack-requirements | 08:26 | |
| *** jpich has joined #openstack-requirements | 09:14 | |
| *** CrayZee has joined #openstack-requirements | 09:48 | |
| *** CrayZee is now known as Guest31212 | 09:48 | |
| *** Guest31212 has quit IRC | 09:49 | |
| *** e0ne has joined #openstack-requirements | 09:50 | |
| *** snapiri- has joined #openstack-requirements | 09:52 | |
| *** dtantsur|afk is now known as dtantsur | 09:55 | |
| *** CrayZee- has joined #openstack-requirements | 09:59 | |
| *** CrayZee- has quit IRC | 09:59 | |
| *** helenafm has joined #openstack-requirements | 10:13 | |
| *** jrist has quit IRC | 10:14 | |
| *** jrist has joined #openstack-requirements | 10:16 | |
| *** snapiri- has quit IRC | 10:19 | |
| *** snapiri- has joined #openstack-requirements | 10:20 | |
| *** CrayZee has joined #openstack-requirements | 10:29 | |
| *** snapiri- has quit IRC | 10:32 | |
| *** efried has quit IRC | 10:38 | |
| *** efried has joined #openstack-requirements | 10:38 | |
| *** CrayZee_ has joined #openstack-requirements | 11:00 | |
| *** CrayZee has quit IRC | 11:02 | |
| *** CrayZee_ has quit IRC | 11:31 | |
| *** dtantsur is now known as dtantsur|brb | 12:32 | |
| *** ccamacho has quit IRC | 12:53 | |
| *** e0ne has quit IRC | 13:03 | |
| *** ccamacho has joined #openstack-requirements | 13:18 | |
| *** vpickard_ is now known as vpickard | 13:28 | |
| *** e0ne has joined #openstack-requirements | 13:37 | |
| *** ccamacho has quit IRC | 13:52 | |
| *** ccamacho has joined #openstack-requirements | 13:53 | |
| *** ccamacho has quit IRC | 14:02 | |
| *** jpich has quit IRC | 14:02 | |
| *** bnemec has quit IRC | 14:02 | |
| *** irclogbot_2 has quit IRC | 14:02 | |
| *** tonyb has quit IRC | 14:02 | |
| *** dmellado has quit IRC | 14:02 | |
| *** mattoliverau has quit IRC | 14:02 | |
| *** dmellado has joined #openstack-requirements | 14:04 | |
| *** jpich has joined #openstack-requirements | 14:04 | |
| *** tonyb has joined #openstack-requirements | 14:07 | |
| *** bnemec has joined #openstack-requirements | 14:07 | |
| *** dtantsur|brb is now known as dtantsur | 15:02 | |
| *** ccamacho has joined #openstack-requirements | 15:09 | |
| *** rajinir has joined #openstack-requirements | 15:42 | |
| efried | bnemec: This is getting ever messier. | 16:20 |
| efried | http://logs.openstack.org/91/616591/1/check/requirements-check/95c6be9/job-output.txt.gz#_2018-11-08_15_53_25_673799 | 16:21 |
| prometheanfire | :| | 16:21 |
| prometheanfire | efried: we've had to force push before, I get the feeling we'll have to do so again | 16:22 |
| efried | prometheanfire: Is that coming from global-requirements.txt | 16:22 |
| prometheanfire | ya | 16:22 |
| efried | so | 16:22 |
| prometheanfire | we don't have the cap, so you can't have the cap iirc | 16:22 |
| efried | we could push a requirements patch to do that cap, then merge the nova cap change, then merge the u-c changes, then merge the nova fix. | 16:23 |
| efried | but | 16:23 |
| efried | I suspect the requirements patch capping g-r would fail against the u-c change that goes above it? | 16:23 |
| efried | maybe not. | 16:23 |
| efried | But at that point, we would be able to merge *another* requirements patch to remove the cap | 16:24 |
| prometheanfire | each change is 'atomic', the uc change would also remove the cap | 16:24 |
| efried | right, and that wouldn't conflict anymore, because it's allowed to come before the nova change. | 16:24 |
| efried | So | 16:24 |
| efried | do we keep chasing this with more patches | 16:24 |
| efried | or do we get out the hammer? | 16:24 |
| efried | will the g-r patch trigger some bot-ness to make the world implement that cap? Not anymore, right? | 16:25 |
| bnemec | So we would need this merge order? g-r cap->nova cap->u-c bump->nova uncap/use fixture | 16:25 |
| efried | bnemec: That sounds correct to me. | 16:26 |
| efried | I'm working up the g-r cap, sigh. | 16:26 |
| prometheanfire | ya, not anymore | 16:26 |
| bnemec | At least most of those are one-line changes. :-/ | 16:26 |
| efried | yeah | 16:26 |
| prometheanfire | I'd rather just have a gr exclusion and not a cap | 16:26 |
| efried | But can we get 'em all through the gate before the summit? :P | 16:27 |
| prometheanfire | we can make it all dependant and do the reviews quicklike | 16:27 |
| bnemec | The queue's been pretty good this week, so there's a chance. :-) | 16:27 |
| openstackgerrit | Eric Fried proposed openstack/requirements master: Cap oslo.service at 1.32.0 (temporarily) https://review.openstack.org/616604 | 16:27 |
| efried | prometheanfire, bnemec: ^ | 16:28 |
| bnemec | Everybody's busy making slides and dinner reservations. ;-) | 16:28 |
| bnemec | efried: prometheanfire wanted an exclusion | 16:28 |
| prometheanfire | I just did a demo of it for work, but yep | 16:30 |
| prometheanfire | I still have to do project update slides | 16:30 |
| efried | prometheanfire: You want !=1.32.1,!=1.33.0 instead of <=1.32.0? | 16:30 |
| prometheanfire | preferably | 16:31 |
| efried | okay, can do... | 16:31 |
| prometheanfire | just because I have a great hatred for caps | 16:31 |
| openstackgerrit | Eric Fried proposed openstack/requirements master: Cap oslo.service at 1.32.0 (temporarily) https://review.openstack.org/616604 | 16:33 |
| efried | prometheanfire: ^ | 16:33 |
| prometheanfire | why is your change failing though? 1.32.0 is currently what we pin against | 16:34 |
| efried | you mean the top one? Because it's using something that's not available until 1.33.0 | 16:34 |
| efried | Let me see if I can do the whole story in 100 words or fewer: | 16:35 |
| prometheanfire | https://review.openstack.org/#/c/615724/ should be able to be merged without a requirements change | 16:37 |
| prometheanfire | because it should be testing against 1.32.0 already (since that's what's in upper-constraints.txt) | 16:38 |
| efried | no, because it uses SleepFixture, which doesn't exist until 1.33.0 | 16:38 |
| prometheanfire | oh, so we will need a force push then I think | 16:38 |
| efried | Well, I suspect this sequence of events would work | 16:39 |
| efried | but a force push would sure be easier. | 16:39 |
| prometheanfire | https://review.openstack.org/#/c/615724 depends on 1.33.0 to be in upper-constraints.txt, we can't update to 1.33.0 til nova cross tests pass | 16:40 |
| prometheanfire | I think this would work better | 16:41 |
| prometheanfire | 1.33.0 uc bump depends on your patch | 16:41 |
| prometheanfire | we merge uc bump, nova is broken without your patch | 16:41 |
| prometheanfire | then your merge nova fix | 16:41 |
| prometheanfire | https://review.openstack.org/#/c/615724 and 1.33.0 are circular (if you are pointing back to us) | 16:42 |
| openstackgerrit | Monty Taylor proposed openstack/requirements master: Add prometheus_client to global requirements https://review.openstack.org/616612 | 16:42 |
| efried | sorry, I didn't quite follow that. Are you suggesting: | 16:42 |
| efried | 1) Force the u-c bump | 16:42 |
| efried | 2) Merge the nova patch | 16:42 |
| efried | ? | 16:42 |
| efried | cause that would work | 16:42 |
| efried | otherwise, we have to do the four-patch dance we're in the middle of. | 16:42 |
| openstackgerrit | Matthew Thode proposed openstack/requirements master: update constraint for oslo.service to new release 1.33.0 https://review.openstack.org/616371 | 16:43 |
| prometheanfire | we won't be force merging | 16:43 |
| prometheanfire | we depend on your nova patch, that means the nova test will pass with it | 16:43 |
| prometheanfire | see ^ review | 16:43 |
| efried | But it's a circular dependency | 16:43 |
| prometheanfire | nope | 16:44 |
| prometheanfire | the depends on in the nova patch points to the oslo.service patch, not the reqs patch | 16:44 |
| efried | yeah, that's currently wrong | 16:44 |
| efried | Are you saying we can do the nova patch without a dep? | 16:45 |
| prometheanfire | yes | 16:45 |
| efried | Maybe I misunderstand Depends-On, but I thought we couldn't merge a dependent without its dependency merging. | 16:45 |
| efried | And the nova patch won't merge until the requirements patch merges. | 16:45 |
| prometheanfire | or maybe I am | 16:46 |
| prometheanfire | ya, I think it's just circular, but if tests pass with it we can then do a force merge (we can't avoid that anyway since there is a circular dep) | 16:46 |
| prometheanfire | another fix would be to readd the funcion to oslo.service, release, have nova make the change then, and then update constraints | 16:47 |
| prometheanfire | that's the way around force | 16:47 |
| efried | Well, we can avoid the circularity with the patches as currently staged, namely: | 16:47 |
| efried | 1) merge the requirements g-r exclusion patch | 16:47 |
| efried | 2) merge the nova requirements exclusion patch | 16:47 |
| efried | 3) merge the requirements u-c bump and exclusion removal | 16:47 |
| efried | 4) merge the nova patch with a) the original fix, b) u-c bump and removal of exclusion | 16:47 |
| prometheanfire | fungi: may have a force merge situation with oslo.service | 16:48 |
| efried | since the patches are already out there, and this requires no forcing, we may as well? | 16:48 |
| *** e0ne has quit IRC | 16:48 | |
| prometheanfire | efried: the gr exclusion patch does nothing because upper-constraints are already set to 1.32.0 | 16:48 |
| *** helenafm has quit IRC | 16:48 | |
| efried | right, it does nothing *except* allow the nova exclusion to go. It was failing the requirements check otherwise. | 16:48 |
| prometheanfire | efried: your number 3 depends on 4 (which depends on 3) | 16:48 |
| efried | no, because requirements u-c changes are allowed (in fact required) to come before project requirements patches that sync to them. | 16:49 |
| efried | which is part of why we're here in the first place. | 16:49 |
| prometheanfire | efried: was it failing on your lower-constraints change? | 16:50 |
| efried | "it" which? | 16:50 |
| prometheanfire | requirements-check | 16:50 |
| efried | on which patch? the real nova fix? | 16:50 |
| prometheanfire | yes | 16:50 |
| efried | It would fail l-c if we didn't have l-c at 1.33.0 because <1.33.0 wouldn't have SleepFixture | 16:51 |
| efried | But it fails requirements-check with l-c/requirements at 1.33.0 without 1.33.0 in u-c | 16:51 |
| efried | unless we depend on the u-c bump patch, which brings in the circularity. | 16:51 |
| prometheanfire | right, that's what I think we can't avoid | 16:52 |
| efried | looklook, if I make the 4-layer dep chain, and they all pass, you'll be sold, yes? | 16:52 |
| prometheanfire | ofc | 16:52 |
| prometheanfire | I didn't see a patch for just 2 | 16:53 |
| prometheanfire | maybe that's what I was missing | 16:53 |
| efried | prometheanfire: that's https://review.openstack.org/#/c/616591/ | 16:53 |
| prometheanfire | I still don't think we can do 3 without 4 though | 16:53 |
| prometheanfire | we can't bump to 1.33.0 while nova is excluding 1.32.1 and 1.33.0 | 16:54 |
| efried | okay, that was the part I wasn't sure about. Though I don't see why not. | 16:54 |
| efried | I guess because the whole point of u-c is to make sure all the projects can run at it? | 16:56 |
| prometheanfire | verifying now | 16:57 |
| prometheanfire | yes, but your requirements are specifically excluding it as well | 16:57 |
| prometheanfire | pip may be stupid and not care about the reqs if a constraint is specified though | 16:57 |
| prometheanfire | cat reqs.txt uc.txt | 16:57 |
| prometheanfire | oslo.service!=1.28.1,>=1.24.0,!=1.32.1,!=1.33.0 | 16:57 |
| prometheanfire | oslo.service===1.33.0 | 16:57 |
| prometheanfire | and pip install -r reqs.txt -c uc.txt worked | 16:58 |
| prometheanfire | so we could try it | 16:58 |
| bnemec | Heh, this might be the first time pip being dumb has worked in our favor. :-) | 16:58 |
| prometheanfire | nova will fail requirements-check gate while 2 is merged and 4 is unmerged possibly | 16:58 |
| prometheanfire | eh, === in constraints override all, it's intended behavior | 16:59 |
| bnemec | Yeah, it's just funny that you can say "don't use 1.33.0...but actually use _only_ 1.33.0". | 17:00 |
| prometheanfire | ya | 17:00 |
| efried | so what's the current plan? | 17:01 |
| efried | 4-step sequence above could be verified-or-not by setting the dep chain and letting zuul do its thing. | 17:01 |
| efried | Do we believe that has a chance of working? | 17:02 |
| prometheanfire | yes | 17:02 |
| efried | okay. I'll set up the dep chains, stand by... | 17:02 |
| openstackgerrit | Eric Fried proposed openstack/requirements master: update constraint for oslo.service to new release 1.33.0 https://review.openstack.org/616371 | 17:03 |
| *** melwitt has joined #openstack-requirements | 17:03 | |
| efried | one more thing, I need to rebase the two nova patches into the same series or they'll merge conflict. | 17:06 |
| efried | actually, we may need to do the same with the reqs patches... | 17:06 |
| openstackgerrit | Eric Fried proposed openstack/requirements master: update constraint for oslo.service to new release 1.33.0 https://review.openstack.org/616371 | 17:09 |
| efried | okay, I think that'll do r | 17:09 |
| openstackgerrit | Eric Fried proposed openstack/requirements master: update constraint for oslo.service to new release 1.33.0 https://review.openstack.org/616371 | 17:11 |
| efried | there | 17:11 |
| efried | melwitt, bnemec, prometheanfire: Okay, we should be good now. Two series, interleaved. Follow the dep chain from https://review.openstack.org/#/c/615724/4 | 17:13 |
| efried | let's see what zuul has to say. | 17:13 |
| efried | um, it's already complaining :( | 17:14 |
| efried | ...but why? | 17:15 |
| efried | we may be able to recheck once the dust has settled. I'm going to go do something else for a while. | 17:16 |
| prometheanfire | ya, I was wondering about having to recheck | 17:25 |
| prometheanfire | This change depends on a change that failed to merge. | 17:25 |
| prometheanfire | rebase? | 17:25 |
| prometheanfire | or dependant patch failed | 17:25 |
| efried | I couldn't find a failed dep. But it may have happened because I reproposed something in the middle. A recheck might do it. | 17:29 |
| efried | even now | 17:29 |
| efried | otherwise, I may have to rebase them in order :( | 17:29 |
| prometheanfire | :| | 17:30 |
| prometheanfire | ya, not sure, it looks ok, the reqs patches might need to stack/rebase though | 17:31 |
| prometheanfire | not sure if you did that | 17:31 |
| bnemec | Yeah, I think if you have a patch that is dependent on another patch, and push a new PS to the dependency the dependent patch needs to get rebased even if it doesn't need changes. | 17:32 |
| bnemec | At least I seem to recall running into that limitation with depends-on before. | 17:32 |
| *** ccamacho has quit IRC | 17:32 | |
| prometheanfire | I leave tomorrow but should have internet most times (if we are lucky this'll get mostly done today | 17:33 |
| prometheanfire | ya, iirc deps-on is good for cross project, but not for intra-project | 17:33 |
| prometheanfire | that still needs the classic stacking iirc | 17:33 |
| prometheanfire | been a while since I've done that too | 17:33 |
| *** jpich has quit IRC | 17:34 | |
| *** irclogbot_2 has joined #openstack-requirements | 18:01 | |
| mordred | prometheanfire: http://logs.openstack.org/12/616612/1/check/openstack-tox-validate/61cf80e/job-output.txt.gz#_2018-11-08_17_07_56_055138 wants me to use a - - but pypi doesn't list it with a - ... is it safe to always use - ? | 18:53 |
| prometheanfire | mordred: whatever you pip install with should be right | 19:05 |
| prometheanfire | mordred: ya, they are equiv to pip | 19:23 |
| *** e0ne has joined #openstack-requirements | 20:19 | |
| *** e0ne has quit IRC | 20:22 | |
| efried | prometheanfire, bnemec: Okay, we're looking good on three out of four, but https://review.openstack.org/#/c/616371/ is failing on the cross-nova job. It's apparently ignoring the exclusion in the nova project's requirements.txt and instead using its own g-r/u-c. | 20:31 |
| efried | Which is a reasonable thing for it to want to do, I suppose, but it puts us back in a bind. | 20:31 |
| efried | prometheanfire, fungi: Since the top patch (https://review.openstack.org/#/c/615724/) is passing, can we force that one -^ ? | 20:32 |
| bnemec | Bleh. Yeah I guess a force merge is probably our best option. | 20:36 |
| efried | I'm sure there's more contortions we could do to get it to go naturally, but we passed the point of diminishing returns long ago. | 20:36 |
| *** mriedem has joined #openstack-requirements | 20:37 | |
| *** mriedem has quit IRC | 20:41 | |
| prometheanfire | dirk: tonyb dhellmann smcginnis mind looking at https://review.openstack.org/616604 ? | 20:44 |
| smcginnis | Looking | 20:46 |
| efried | prometheanfire, bnemec: Ohh, actually it looks like it's because it's only installing nova's *test-*requirements.txt http://logs.openstack.org/71/616371/5/check/cross-nova-py27/9092c83/job-output.txt.gz#_2018-11-08_17_37_49_365402 | 20:46 |
| prometheanfire | huh | 20:47 |
| efried | ...which may actually be a bug in the job? | 20:47 |
| smcginnis | efried: Are you saying the cap of oslo.service is not needed? | 20:47 |
| efried | smcginnis: I'm saying it is either needed in test-requirements, or the job ought to be installing deps from both requirements.txt and test-requirements.txt | 20:48 |
| smcginnis | Usually jobs need to do both I think. | 20:48 |
| efried | I would have thought so. | 20:48 |
| smcginnis | efried: But to be clear, we should not merge the cap? | 20:49 |
| efried | nova doesn't have oslo.service in test-requirements because it's used in requirements | 20:49 |
| prometheanfire | is it using constraints? | 20:49 |
| efried | smcginnis: Which cap? | 20:49 |
| smcginnis | efried: prometheanfire just pinged to look at https://review.openstack.org/#/c/616604/. I wasn't sure if you were responding to that, or if this was something else. | 20:50 |
| efried | prometheanfire: Which constraints? | 20:50 |
| smcginnis | prometheanfire: Yeah - https://github.com/openstack/nova/blob/master/tox.ini#L14 | 20:50 |
| efried | smcginnis: Okay, yeah, this whole pile is interrelated. I can give you the back story if you like. | 20:51 |
| efried | smcginnis: It's explained in the next patch, https://review.openstack.org/#/c/616591/ | 20:51 |
| dhellmann | I think I'm going to need that story | 20:51 |
| efried | okay, here goes: | 20:51 |
| dhellmann | it's not clear why we need to exclude in nova explicitly, what job is failing without that? | 20:51 |
| dhellmann | what is the "sniff test" mentioned in https://review.openstack.org/#/c/616591/? | 20:52 |
| * bnemec makes popcorn | 20:53 | |
| bnemec | It's an epic story. :-) | 20:53 |
| dhellmann | fwiw, I'm just unclear, not arguing that any of this is the wrong approach | 20:54 |
| efried | dhellmann: Okay, composed the epic. Here it comes: | 21:03 |
| efried | 1. Nova was mocking something private in oslo.service (yeah, my bad) | 21:03 |
| efried | 2. oslo.service removed the path to the private thing. <== that was released in 1.32.0 | 21:03 |
| efried | 3. When requirements bot tried to bump u-c to 1.32.0, it failed in the cross-nova jobs because that mock was referencing a gone thing, so melwitt proposed https://review.openstack.org/#/c/615724/1 | 21:03 |
| efried | 4. dhellmann (justifiably) complained that ^ was just kicking the can down the road by mocking a *different* private thing, so I created SleepFixture via https://review.openstack.org/#/c/615978/ and it was released via 1.33.0 | 21:03 |
| efried | 5. We updated the patch from 3 to use SleepFixture and depend on 1.33.0 via PS2 (https://review.openstack.org/#/c/615724/2). | 21:03 |
| efried | 6. That was doomed to failure because circular dependency. So we created a patch in nova to constrain oslo.service to <=1.32.0: https://review.openstack.org/#/c/616591/, the theory being that the requirements u-c bump to 1.33.0 could depend on THAT, merge, and leave the way open for the nova patch to merge. | 21:03 |
| efried | 7. But that one bounced on the requirements job because the constraint wasn't in g-r. So we proposed a requirements patch with the same constraint - https://review.openstack.org/#/c/616604/ - on the theory that we could merge all four in the proper order. | 21:03 |
| efried | 8. So now we have four patches in a dependency chain. From bottom (first-to-merge) to top: | 21:03 |
| efried | a openstack/requirements caps oslo.service to 1.32.0: https://review.openstack.org/#/c/616604/ | 21:03 |
| efried | b openstack/nova caps oslo.service likewise: https://review.openstack.org/#/c/616591/ | 21:03 |
| efried | c openstack/requirements removes the cap and bumps u-c to 1.33.0: https://review.openstack.org/#/c/616371/ | 21:03 |
| efried | d openstack/nova original patch to use the new fixture, remove its cap, and bump its minimum to 1.33.0: https://review.openstack.org/#/c/615724/ | 21:03 |
| efried | 9. a, b, and d are passing zuul, but c is failing because the cross-nova jobs are *only* installing nova's test-requirements.txt, not its requirements.txt. The oslo.service line is in requirements.txt and not test-requirements.txt, which is the right thing. So we could either change b to add the line temporarily to test-requirements.txt, and then rip it out in d; or we could force-merge c and get on with our lives. | 21:03 |
| efried | And btw, I don't know that the former would even work - it might blow up because of the "conflict" between g-r/u-c and nova's req. | 21:04 |
| efried | EOM | 21:04 |
| dhellmann | sorry, stepped away | 21:09 |
| dhellmann | efried : doesn't the test in C also install nova itself? | 21:09 |
| dhellmann | it runs the unit tests, right? | 21:09 |
| efried | Yes, it runs the py27 and py35 unit test suites. | 21:09 |
| dhellmann | how does it run those without installing nova, and therefore the contents of requirements.txt? | 21:09 |
| efried | dhellmann: I assume because it's installing the contents of upper-constraints.txt | 21:10 |
| dhellmann | maybe I misunderstood "the cross-nova jobs are *only* installing nova's test-requirements.txt" | 21:10 |
| dhellmann | it uses the constraints file to constrain things; it doesn't install all of the items listed there | 21:10 |
| dhellmann | where's the log from the failure of that cross-nova job? | 21:10 |
| efried | ... | 21:10 |
| dhellmann | got it | 21:11 |
| efried | http://logs.openstack.org/71/616371/5/check/cross-nova-py27/9092c83/ | 21:11 |
| dhellmann | http://logs.openstack.org/71/616371/5/check/cross-nova-py27/9092c83/tox/py27-2.log shows it in stalling nova | 21:12 |
| dhellmann | with the constraints file | 21:12 |
| dhellmann | and taking 1.33.0 | 21:12 |
| dhellmann | that's the version we want, right? | 21:12 |
| dhellmann | ok, I think I see what's going on | 21:13 |
| dhellmann | we can't land the constraint patch because nova master doesn't work with that version of the lib because that version of the lib still doesn't have the private thing nova's tests want to mock | 21:14 |
| dhellmann | and we can't land the change to nova to fix that because the constraints list is using a version of the library without the fixture | 21:14 |
| dhellmann | how about a change to nova to disable that test, then we land the constraint update, then re-enable the test? | 21:14 |
| dhellmann | that feels simpler than fiddling with exclusions | 21:15 |
| dhellmann | and we can line them all up with depends-on to verify that the scheme works before approving any of it | 21:15 |
| efried | Yeah, we could do that. But we've spent hours on this already and have four patches in flight. The top one, which represents where we want to end up, is passing tests. | 21:15 |
| efried | So at this point I'm looking for the shortest path. | 21:15 |
| dhellmann | oh, sorry, I thought this set of patches still wasn't working | 21:16 |
| dhellmann | yeah, sorry | 21:16 |
| efried | c isn't working, but d is. | 21:16 |
| efried | and c isn't working because it's installing 1.33.0, despite nova's restriction in requirements.txt | 21:16 |
| efried | If c installed 1.32.0 instead, which is what it would be doing if it were paying attention to nova's requirements.txt, it would be passing too. | 21:16 |
| efried | At this point the proposal is to force c and go home. | 21:17 |
| dhellmann | is https://review.openstack.org/#/c/616371/5 what you're calling C? | 21:17 |
| efried | yes | 21:17 |
| dhellmann | that's installing 1.33.0 because that's what the patch tells it to do | 21:17 |
| dhellmann | and that's not going to work with depends-on because the unit test jobs don't know how to do depends-on | 21:17 |
| efried | well, right, I think what it's doing is understandable, but it's arguable that it sholud go the othre way too. | 21:17 |
| dhellmann | so we need to start from the bottom of the pile and then recheck | 21:17 |
| efried | um, the ut jobs must be able to handle depends-on, or d wouldn't be passing requirements jobs. | 21:18 |
| dhellmann | D is running different jobs | 21:18 |
| dhellmann | the unit test jobs in the nova repo can take advantage of depends-on to get an updated requirements list | 21:18 |
| efried | at one point it failed requirements check precisely because c hadn't landed yet | 21:18 |
| dhellmann | I'm not sure the cross jobs are smart enough to use depends-on in that way | 21:19 |
| dhellmann | right, my point is just that not all jobs actually know how to use depends-on | 21:19 |
| dhellmann | it looks like tonyb just approve A | 21:20 |
| dhellmann | and mriedem has approved B | 21:20 |
| dhellmann | I would let those merge and then recheck C | 21:21 |
| efried | And you think that'll make C install 1.32.0 instead of 1.33.0? | 21:23 |
| dhellmann | efried : fwiw, the constraints engine in pip overrides any other settings. It installs exactly the version specified in the constraint | 21:23 |
| dhellmann | so, no | 21:23 |
| efried | What's "the constraint" in this scenario? | 21:23 |
| dhellmann | the entry in upper-constraints.txt | 21:24 |
| efried | ah | 21:24 |
| efried | okay, then there's no way this works. | 21:24 |
| efried | without either forcing or making a new patch to remove the failing tests. | 21:24 |
| efried | or temporarily mock the new private thing as was done in PS1 | 21:24 |
| dhellmann | how many tests fail because of this? | 21:24 |
| dhellmann | it looked like several | 21:25 |
| efried | Yeah, but the surface area isn't all that high. I think commenting out ~3LOC would do the trick. | 21:26 |
| dhellmann | it looks like 3 classes, though | 21:26 |
| efried | The mock isn't necessary to make the tests pass. It just makes them not incur real sleeps. | 21:26 |
| efried | so they take longer without it. | 21:26 |
| efried | But not like weeks or anything. It's just a few seconds. | 21:26 |
| dhellmann | I suppose that's safer than having them not run at all | 21:26 |
| efried | Is that how you want to do this, a new nova patch and abandon the two exclusion patches? | 21:27 |
| * dhellmann thinks | 21:28 | |
| efried | include in your thinking that I've already spent basically a full day on this, for what amounts to 4LOC. | 21:29 |
| dhellmann | I guess there's no way to land the "good" test fix in nova without updating the constraint | 21:29 |
| efried | I know I'm being punished for mocking the private in the first place, but... | 21:29 |
| efried | correct. | 21:29 |
| dhellmann | yeah, I wish I had realized you were so bound up in it or I would have tried to advise earlier | 21:29 |
| dhellmann | you'll remember not to do this in future :-) | 21:29 |
| dhellmann | I think I would leave the exclusion patches in play | 21:29 |
| dhellmann | there's no sense in resetting the gate now | 21:29 |
| dhellmann | that's just going to add more delay | 21:30 |
| dhellmann | I think the next patch we need is one in nova to either disable the tests or make them not do the mock; whichever you prefer and can make work | 21:30 |
| efried | ... | 21:30 |
| dhellmann | after that it should be possible to land C and D | 21:30 |
| dhellmann | and then in one of those you can re-enable the tests | 21:30 |
| efried | ugh, I'm in the middle of a restack in my nova repo, gonna be a bit before I could get to that, boo. | 21:31 |
| dhellmann | I guess that means changing the contents of D | 21:31 |
| efried | It means rebasing D on the new patch (inserting it in between the two in that series) and making c depends-on it. | 21:31 |
| dhellmann | would it be helpful if I tried my hand at writing that missing patch? | 21:32 |
| dhellmann | or is that just going to confuse things further | 21:32 |
| efried | dhellmann: Yeah, very much so. It's easy, just comment out those three mocks, I'll show ya... | 21:32 |
| dhellmann | I can find them ia 615724 | 21:33 |
| dhellmann | via | 21:33 |
| efried | https://review.openstack.org/#/c/615724/1 | 21:33 |
| efried | Or actually just *use* that (PS1) | 21:33 |
| efried | which mocks new privates, but would also do the trick. | 21:33 |
| efried | but obviously don't revert that change to PS1, because PS4 is what we actually need. | 21:34 |
| efried | except when it gets rebased on the new patch, the delta to requirements.txt will need to be resolved (the end result will be the same). | 21:35 |
| dhellmann | efried : see how you like https://review.openstack.org/616697 | 21:39 |
| openstackgerrit | Monty Taylor proposed openstack/requirements master: Add prometheus_client to global requirements https://review.openstack.org/616612 | 21:40 |
| dhellmann | ah, bummer, I rebased the one at the bottom of the stack and that kicked it out of the gate | 21:41 |
| dhellmann | sorry about that :-/ | 21:41 |
| efried | meh | 21:41 |
| efried | I guess at this point we could do without it, eh | 21:41 |
| dhellmann | that's true | 21:41 |
| dhellmann | one less thing to land | 21:41 |
| efried | If you want to rebase the other two, then re-Depends-On the req u-c bump (https://review.openstack.org/#/c/616371/) to the nova patch... | 21:42 |
| dhellmann | rebase the other 2? | 21:42 |
| efried | dhellmann: I'll wait for that ^ and then I'll push the nova patches | 21:42 |
| dhellmann | we can edit https://review.openstack.org/#/c/616371/5 through the web to add the depends-on | 21:43 |
| efried | yeah, so https://review.openstack.org/#/c/615724/5 is the top of a 3-parter, and we don't need the bottom part. So rebase (or I think it'll actually be a cherry-pick?) the top two onto master to exclude the bottom one, which can be abandoned. | 21:43 |
| openstackgerrit | Doug Hellmann proposed openstack/requirements master: update constraint for oslo.service to new release 1.33.0 https://review.openstack.org/616371 | 21:43 |
| efried | ...but do it before you do that ^ :P | 21:43 |
| efried | or I can push the bottom one since it's got two +2s and we can keep the pile the way it is (but will have to babysit it more). | 21:44 |
| dhellmann | https://review.openstack.org/#/c/616371/6 has 2 depends-on, the 1 I added for the "stop mocking" patch to nova and https://review.openstack.org/#/c/616591/ which is the first patch to nova to fiddle the caps | 21:44 |
| dhellmann | I guess you're suggesting remove the changes that try to cap and uncap? | 21:44 |
| dhellmann | I think either approach would work | 21:45 |
| efried | the change (singular) that tries to cap | 21:45 |
| dhellmann | but removing patches we know we don't need means less wait time | 21:45 |
| efried | which will require trivial merge conflict resolution on requirements.txt on the top patch (to make it look just like it does now) | 21:45 |
| efried | yes, agree. We could even rip the req proj capper out of the gate | 21:46 |
| efried | https://review.openstack.org/#/c/616604/ <== by rebasing this | 21:46 |
| efried | and then abandoning it. | 21:46 |
| dhellmann | ok, here's the 5 patches I see: https://review.openstack.org/#/q/topic:loopingcall-wait+(status:open+OR+status:merged) | 21:46 |
| dhellmann | yeah, I think we can remove 616604 | 21:47 |
| efried | okay... | 21:47 |
| openstackgerrit | Doug Hellmann proposed openstack/requirements master: update constraint for oslo.service to new release 1.33.0 https://review.openstack.org/616371 | 21:47 |
| efried | does 'abandon' yank it out of the gate? | 21:47 |
| dhellmann | that should remove it from the dependency chain | 21:47 |
| efried | cause there's nothing for it to be rebased on. | 21:48 |
| dhellmann | I think we can also abandon https://review.openstack.org/#/c/616591/ | 21:48 |
| efried | both done. | 21:49 |
| dhellmann | ok, so now we need to rebase the other 2 that were on top of 616591 | 21:49 |
| efried | yes | 21:49 |
| dhellmann | because those are the 2 in nova that we need | 21:49 |
| efried | yes | 21:49 |
| dhellmann | do you want to do that, or shall I? | 21:49 |
| efried | You please | 21:49 |
| efried | cause my nova repo is in use still | 21:50 |
| efried | And I can't rebase from the web UI unless I go look up what the master branch's latest change set is called. | 21:50 |
| dhellmann | ok, done | 21:51 |
| dhellmann | now let me check the depends-on chain here | 21:51 |
| dhellmann | we want https://review.openstack.org/#/c/616697/ first and it has no depends-on | 21:51 |
| dhellmann | good | 21:51 |
| dhellmann | we want https://review.openstack.org/#/c/616371/ second | 21:52 |
| dhellmann | that depends on https://review.openstack.org/#/c/616591/ which is abandoned, so bad, let me fix that | 21:52 |
| efried | https://review.openstack.org/#/c/616371/ needs to be rebased now. | 21:52 |
| openstackgerrit | Doug Hellmann proposed openstack/requirements master: update constraint for oslo.service to new release 1.33.0 https://review.openstack.org/616371 | 21:52 |
| efried | oh, yeah, that'd do it. | 21:52 |
| efried | you can remove the extra line in the commit message too. | 21:52 |
| openstackgerrit | Doug Hellmann proposed openstack/requirements master: update constraint for oslo.service to new release 1.33.0 https://review.openstack.org/616371 | 21:53 |
| dhellmann | done | 21:53 |
| dhellmann | and now https://review.openstack.org/#/c/615724/ which depends on 616371 which is correct | 21:54 |
| efried | Who's gonna approve https://review.openstack.org/#/c/616371/ for us? prometheanfire still around? | 21:54 |
| efried | and/or tonyb? | 21:54 |
| dhellmann | hmm, I wonder what we need to do to make zuul happy with https://review.openstack.org/#/c/615724/6 | 21:55 |
| dhellmann | we may have to edit that patch | 21:55 |
| efried | recheck did it last time | 21:55 |
| dhellmann | oh, ok, I tried that and it didn't seem to have any effect | 21:55 |
| dhellmann | maybe I just wasn't patient enough | 21:55 |
| efried | Did you rebase it by hand? | 21:56 |
| tonyb | efried: if it passes zuul befoer I get on a plane I'll +2+W it | 21:56 |
| efried | tonyb: ack, thx | 21:57 |
| dhellmann | tonyb : I went ahead and approved it; it's a bot update and the other changes to it are incidental | 21:57 |
| tonyb | dhellmann: Okay | 21:57 |
| dhellmann | efried : I have rebased 615724 by hand, but only once I think? | 21:57 |
| dhellmann | oh, that's currently in the check queue so maybe after the jobs run zuul will update that merge conflict status | 21:58 |
| dhellmann | efried : I think you're good to go. if 615724 doesn't end up in a state that can be merged you can try rebasing it | 21:59 |
| efried | ack | 21:59 |
| dhellmann | sorry again you spent so much time on this -- if something like this comes up again drop in here and we can work out a plan earlier in the process | 21:59 |
| efried | dhellmann: Not sure it would have gone a lot better tbh. I'm sure we still would have tried figuring out a "pure" way to fix it - i.e. without any intermediate patches "losing" anything. | 22:01 |
| dhellmann | oh, no, I would have suggested that right away :-) | 22:02 |
| dhellmann | that's usually the easiest way to fix these sorts of problems | 22:03 |
| prometheanfire | packing | 22:03 |
| efried | prometheanfire: Thanks for the help today. You too bnemec dhellmann | 22:04 |
| *** mattoliverau has joined #openstack-requirements | 22:06 | |
| *** vpickard is now known as vpickard_ | 22:50 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!