| *** Shuo_ has quit IRC | 00:02 | |
| *** harlowja has joined #zuul | 00:06 | |
| *** jkt has quit IRC | 01:27 | |
| *** jkt has joined #zuul | 01:29 | |
| *** jhesketh has quit IRC | 01:36 | |
| *** jhesketh has joined #zuul | 01:36 | |
| *** harlowja has quit IRC | 02:36 | |
| *** bhavik1 has joined #zuul | 05:38 | |
| *** bhavik1 has quit IRC | 05:49 | |
| *** abregman has joined #zuul | 05:55 | |
| *** saneax-_-|AFK is now known as saneax | 07:00 | |
| *** abregman has quit IRC | 08:27 | |
| *** abregman has joined #zuul | 08:29 | |
| *** hashar has joined #zuul | 08:41 | |
| *** abregman has quit IRC | 08:50 | |
| *** abregman has joined #zuul | 08:53 | |
| *** jhesketh has quit IRC | 09:27 | |
| *** jhesketh has joined #zuul | 09:30 | |
| *** abregman is now known as abregman|afk | 11:39 | |
| *** bhavik1 has joined #zuul | 12:13 | |
| *** bhavik1 has quit IRC | 12:21 | |
| *** jamielennox is now known as jamielennox|away | 12:59 | |
| *** jamielennox|away is now known as jamielennox | 13:19 | |
| *** abregman|afk is now known as abregman | 13:28 | |
| *** abregman is now known as abregman|mtg | 14:28 | |
| *** yolanda has quit IRC | 14:40 | |
| *** yolanda has joined #zuul | 14:41 | |
| *** openstack has joined #zuul | 15:21 | |
| *** mgagne has joined #zuul | 15:22 | |
| *** mgagne has quit IRC | 15:23 | |
| *** mgagne has joined #zuul | 15:23 | |
| *** mgagne is now known as Guest58531 | 15:23 | |
| *** zaro has joined #zuul | 15:24 | |
| *** saneax is now known as saneax-_-|AFK | 15:36 | |
| *** abregman|mtg has quit IRC | 15:58 | |
| *** abregman has joined #zuul | 16:01 | |
| *** hashar has quit IRC | 16:02 | |
| *** hashar has joined #zuul | 16:31 | |
| *** harlowja has joined #zuul | 16:39 | |
| *** abregman has quit IRC | 16:47 | |
| jeblair | is anyone else interested in reviewing the nodepool changes for zuul, or should i just go ahead and merge them? that's the stack that starts at 413296 and runs through 417471 | 16:52 |
|---|---|---|
| SpamapS | jeblair: I can take a gander, if nothing else so I can learn :) | 16:56 |
| jeblair | SpamapS: thanks :) | 16:56 |
| Shrews | jeblair: think i already covered the ones with ZK interaction, but i'll double check | 16:57 |
| SpamapS | jeblair: is 413296 the only thing holding the stack back? | 16:58 |
| jeblair | Shrews: yeah, i think i see your name on most of those, except maybe a few near the end? | 16:58 |
| mordred | jeblair: oh - crap - sorry - reviewing now | 17:02 |
| jeblair | SpamapS: well it looks like this: http://paste.openstack.org/show/595434/ | 17:02 |
| jeblair | SpamapS: so yes, but only the next 3 after it have been approved | 17:02 |
| mordred | jeblair: did you see Shrews comment in https://review.openstack.org/#/c/414382/1/tests/test_nodepool.py ? (it can be fixed in a followup) | 17:04 |
| jeblair | mordred: i have not, thx | 17:04 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul: Fix copyright date in test_nodepool https://review.openstack.org/422111 | 17:05 |
| * SpamapS is learning stuff | 17:07 | |
| Shrews | jeblair: did you mean for returnNodeSet() to *always* update the node? | 17:08 |
| Shrews | jeblair: seems like you'd only do that if you set state to 'used' | 17:08 |
| Shrews | https://review.openstack.org/#/c/416739/6/zuul/nodepool.py | 17:08 |
| jeblair | Shrews: agreed -- i can't think of a reason to store the node if the state didn't update. i should move it into that conditional block. | 17:10 |
| openstackgerrit | Merged openstack-infra/zuul: Add FakeNodepool test fixture https://review.openstack.org/413296 | 17:11 |
| jeblair | Shrews: i'm going make a followup for that | 17:11 |
| jeblair | Shrews: ha! it's already fixed in a later patch :) | 17:11 |
| Shrews | jeblair: that's fine. already left a -1, but cores can ignore that | 17:11 |
| Shrews | jeblair: ah, well i'll change my vote then | 17:11 |
| openstackgerrit | Merged openstack-infra/zuul: Fix race condition in test_success_url https://review.openstack.org/413811 | 17:12 |
| jeblair | Shrews: the next patch, in fact | 17:12 |
| openstackgerrit | Merged openstack-infra/zuul: Improve test output https://review.openstack.org/413812 | 17:12 |
| openstackgerrit | Merged openstack-infra/zuul: Add a test printHistory function https://review.openstack.org/414378 | 17:13 |
| jeblair | i responded in a comment on 416739 for the record | 17:13 |
| SpamapS | left a comment on 413296 .. just wondering if FakeNodepool should use a different request root. | 17:17 |
| SpamapS | (just to avoid accidental test suite interaction with real nodepools) | 17:17 |
| mordred | jeblair: also - https://review.openstack.org/#/c/416700/6/zuul/model.py comments from Shrews there. just want to make sure you saw them | 17:19 |
| jeblair | SpamapS: yes indeed! that all runs under a zookeeper test fixture with a chroot set, so there shouldn't be any leakage outside the test fixture's root | 17:20 |
| jeblair | mordred: thx | 17:22 |
| SpamapS | I didn't notice the test fixture chroot. mmmkay. | 17:25 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul: Spell NodeSet consistently https://review.openstack.org/422128 | 17:26 |
| Shrews | jeblair: ahhhh, 422128 silences my OCD alarm | 17:29 |
| * Shrews hits snooze | 17:29 | |
| jeblair | SpamapS: it's here: http://git.openstack.org/cgit/openstack-infra/zuul/tree/tests/base.py?h=feature/zuulv3#n933 | 17:30 |
| *** harlowja has quit IRC | 17:33 | |
| openstackgerrit | Merged openstack-infra/zuul: Add nodepool test class https://review.openstack.org/414382 | 17:45 |
| *** hashar is now known as hasharAway | 18:03 | |
| *** yolanda has quit IRC | 18:06 | |
| *** yolanda has joined #zuul | 18:06 | |
| jeblair | mordred, Shrews, SpamapS: are you finished reviewing or should i wait longer? | 18:11 |
| SpamapS | jeblair: oh, I gave up when things started merging. Sorry. | 18:13 |
| SpamapS | It just convinced me I need to do a bit more formal educating of myself on the nuts and bolts of nodepool | 18:13 |
| jeblair | SpamapS: you may be interested in the flip side of the process happening in 421485 | 18:16 |
| mordred | jeblair: k. stack looks good - I think there's a couple of places that need a +A to be triggered | 18:22 |
| jeblair | mordred: thanks... if no one objects i'll pull the trigger on any remaining around 19:00 | 18:24 |
| Shrews | jeblair: yeah done | 18:27 |
| *** hasharAway has quit IRC | 18:43 | |
| *** hashar has joined #zuul | 18:57 | |
| jlk | jeblair: More questions on pipeline and trigger requirements. I'm looking at the pipline https://github.com/openstack-infra/zuul/blob/master/tests/fixtures/layout-requirement-vote2.yaml#L2-L7 and the test that uses it https://github.com/openstack-infra/zuul/blob/master/tests/test_requirements.py#L215-L260 | 19:18 |
| jlk | jeblair: specifically, https://github.com/openstack-infra/zuul/blob/master/tests/test_requirements.py#L248-L254 | 19:18 |
| jlk | jeblair: I don't understand, from the requirement of username: jenkins and verified [1,2] why a +2 vote from non-jenkins allows the pipeline to trigger | 19:19 |
| openstackgerrit | Merged openstack-infra/zuul: Re-submit node requests on ZooKeeper disconnect https://review.openstack.org/416357 | 19:20 |
| openstackgerrit | Merged openstack-infra/zuul: Copy nodeset when making node requests https://review.openstack.org/416772 | 19:22 |
| openstackgerrit | Merged openstack-infra/zuul: Lock nodes when nodepool request is fulfilled https://review.openstack.org/416700 | 19:22 |
| openstackgerrit | Merged openstack-infra/zuul: Mark nodes as 'in-use' before launching jobs https://review.openstack.org/416737 | 19:23 |
| *** abregman_ has joined #zuul | 19:26 | |
| jeblair | jlk: basically, the trigger requirement specifies characteristics of the triggering event. in this case, only that the event is 'comment-added'. the pipeline requirement specifies characteristics of the change itself regardless of the event, in this case, that it have a positive approval from jenkins. | 19:28 |
| jlk | jeblair: right, so | 19:28 |
| jlk | jeblair: comment-added is potentially triggering, that's fine | 19:29 |
| jlk | but it's a new PR, and the only comment added was a 'nobody' +2 approval | 19:29 |
| jlk | yet the test things that it should meet the requirements, which is a jenkins 1, or 2 | 19:29 |
| jlk | so how does 'nobody' +2 meet that? | 19:29 |
| mordred | jlk: right before the nobody +2 it has a jenkins verify +1 | 19:32 |
| mordred | jlk: line 241-246 | 19:32 |
| jlk | mordred: that's the A change, not the B change | 19:33 |
| jlk | line 248 starts a whole new change | 19:33 |
| jeblair | hrm, yeah, i don't immediately see the answer. :) | 19:33 |
| mordred | oh! | 19:33 |
| jlk | like, are tests passing, but the code is broken? | 19:33 |
| mordred | jlk: yes - I can see that now | 19:33 |
| openstackgerrit | Merged openstack-infra/zuul: Return nodes after use https://review.openstack.org/416739 | 19:34 |
| jlk | or are the test broken in someway that allows them to pass, but the code is doing the right thing? | 19:34 |
| jlk | (more likely the latter than the former) | 19:34 |
| jeblair | and it's interesting that both variants pass (the 'pipeline' variant of the test as well as the 'trigger' variant) | 19:35 |
| mordred | oh - wait - look at the self.history counts | 19:35 |
| jlk | oooooh | 19:35 |
| jlk | maybe the comments are wrong | 19:35 |
| jeblair | mordred, jlk: yeah, i think that's it :) | 19:35 |
| jlk | and history is still 1, from the previous jobs | 19:35 |
| jlk | so history hasn't increased yet, until line 256 | 19:36 |
| mordred | yup. it doesn't actually get enqueued until the jenkins comment | 19:36 |
| jeblair | right, history is persistent throughout the test. | 19:36 |
| mordred | yah | 19:36 |
| jlk | okay, whew! | 19:36 |
| jeblair | poorly tested comments ;) | 19:36 |
| mordred | I think the comments there could improve :) | 19:36 |
| jlk | related, in 2.5, looks like the requirements are pretty hardcoded into the model, and they're just gerrit things. Adding new github things is going to be.. challenging. | 19:37 |
| jlk | In v3, is this more modular and able to be defined by the connection? | 19:37 |
| jeblair | jlk: not yet, but i think it should be. | 19:37 |
| jeblair | jlk: want to make a story for that? it could be built on top of 418554 | 19:38 |
| * jlk looks | 19:38 | |
| jeblair | there's also a 'ChangeMatcher' class that a lot of this stuff is starting to be derived from, so that could be a good base for the interface | 19:39 |
| jeblair | drivers could define matchers which can be attached either to pipelines or triggers | 19:40 |
| jlk | is that a storyboard or a review? | 19:40 |
| jeblair | oh sorry | 19:40 |
| jeblair | that's a change in gerrit | 19:41 |
| jeblair | i don't think there's an existing story related to this | 19:41 |
| jeblair | i was thinking ahead to implementation :) | 19:41 |
| jlk | so should I toss this up as a story then? | 19:42 |
| jeblair | jlk: yes please | 19:42 |
| jlk | can do | 19:42 |
| jlk | https://storyboard.openstack.org/?#!/story/2000843 | 19:46 |
| openstackgerrit | Merged openstack-infra/zuul: Cancel/return nodepool requests on job cancel https://review.openstack.org/416795 | 20:10 |
| openstackgerrit | Merged openstack-infra/zuul: Remove unused clasess from zk.py https://review.openstack.org/417091 | 20:10 |
| *** abregman_ has quit IRC | 20:28 | |
| openstackgerrit | Merged openstack-infra/zuul: Remove excess printing from stats test https://review.openstack.org/417200 | 21:24 |
| openstackgerrit | Merged openstack-infra/zuul: Verify nodes and requests are not leaked https://review.openstack.org/417136 | 21:24 |
| *** jamielennox is now known as jamielennox|away | 21:26 | |
| *** jamielennox|away is now known as jamielennox | 22:10 | |
| openstackgerrit | Merged openstack-infra/zuul: Use constants for state names https://review.openstack.org/417470 | 22:12 |
| openstackgerrit | Merged openstack-infra/zuul: Handle nodepool allocation failure https://review.openstack.org/417471 | 22:14 |
| openstackgerrit | Merged openstack-infra/zuul: Fix copyright date in test_nodepool https://review.openstack.org/422111 | 22:14 |
| openstackgerrit | Merged openstack-infra/zuul: Spell NodeSet consistently https://review.openstack.org/422128 | 22:14 |
| *** jamielennox is now known as jamielennox|away | 22:24 | |
| openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Add --fake command line option to builder https://review.openstack.org/422283 | 22:29 |
| openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Add files for zuul-nodepool integration test https://review.openstack.org/422284 | 22:29 |
| jeblair | Shrews: ^ | 22:30 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul: Add nodepool integration test script https://review.openstack.org/422288 | 22:36 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul: Add nodepool integration test script https://review.openstack.org/422288 | 22:42 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul: Add nodepool integration test script https://review.openstack.org/422288 | 22:58 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul: Add nodepool integration test script https://review.openstack.org/422288 | 23:00 |
| *** saneax-_-|AFK is now known as saneax | 23:15 | |
| *** hashar has quit IRC | 23:22 | |
| *** fungi has quit IRC | 23:24 | |
| *** jkt has quit IRC | 23:24 | |
| *** fungi has joined #zuul | 23:24 | |
| *** jkt has joined #zuul | 23:24 | |
| jlk | jeblair: question, is there any way at all to have one pipeline success be a potential trigger for another pipeline? Some internal way of signaling that "since this pipeline finished, maybe try that one too?" | 23:54 |
| jlk | jeblair: instead of looping through the external connection service? | 23:54 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!