Friday, 2018-06-01

*** Wei_Liu has quit IRC00:19
openstackgerritTristan Cacqueray proposed openstack-infra/zuul master: docs: add Project Testing Interface guide  https://review.openstack.org/57142000:21
mnaseri know not everyone is a fan of js reviews but00:24
mnaserhttps://review.openstack.org/#/q/topic:fix-status+project:openstack-infra/zuul00:24
*** harlowja has quit IRC00:58
*** Wei_Liu has joined #zuul01:03
*** mordred has quit IRC02:41
*** Shrews has quit IRC02:45
*** Shrews has joined #zuul02:47
*** mordred has joined #zuul02:54
*** rlandy|rover|bbl is now known as rlandy|rover03:31
*** EmilienM_ has joined #zuul03:44
*** dtruong_ has quit IRC03:44
*** fdegir has quit IRC03:44
*** dvn has quit IRC03:44
*** jamielennox has quit IRC03:44
*** robled has quit IRC03:44
*** zxiiro has quit IRC03:44
*** jbryce has quit IRC03:44
*** Diabelko has quit IRC03:44
*** EmilienM has quit IRC03:44
*** tristanC has quit IRC03:44
*** EmilienM_ is now known as EmilienM03:44
*** EmilienM has quit IRC03:46
*** EmilienM has joined #zuul03:46
*** toabctl has quit IRC03:47
*** toabctl has joined #zuul03:49
*** rlandy|rover has quit IRC03:59
*** harlowja has joined #zuul04:17
*** harlowja has quit IRC04:20
*** bhavik1 has joined #zuul04:36
*** bhavik1 has quit IRC06:08
*** fdegir has joined #zuul06:22
*** hashar has joined #zuul06:30
*** sshnaidm_pto has joined #zuul06:35
*** pcaruana has joined #zuul06:40
*** jpena|off is now known as jpena07:04
*** gtema has joined #zuul07:16
*** D3VIATION has joined #zuul07:23
*** dtruong_ has joined #zuul07:47
*** dvn has joined #zuul07:47
*** jamielennox has joined #zuul07:47
*** zxiiro has joined #zuul07:47
*** robled has joined #zuul07:47
*** jbryce has joined #zuul07:47
*** Diabelko has joined #zuul07:47
*** tristanC has joined #zuul07:47
*** ssbarnea_ has joined #zuul07:56
*** sshnaidm_pto has quit IRC08:29
*** sshnaidm_pto has joined #zuul08:30
*** sshnaidm_pto has quit IRC08:36
*** electrofelix has joined #zuul08:37
*** sshnaidm_pto has joined #zuul08:51
*** sshnaidm_pto has quit IRC09:20
*** corvus has quit IRC09:33
*** corvus has joined #zuul09:34
*** D3VIATION has quit IRC09:53
*** jpena is now known as jpena|off09:58
*** ssbarnea_ has quit IRC10:25
*** sshnaidm_pto has joined #zuul11:26
*** GonZo2000 has joined #zuul11:31
*** GonZo2000 has quit IRC11:31
*** GonZo2000 has joined #zuul11:31
*** GonZo2000 has quit IRC11:39
*** GonZo2000 has joined #zuul11:42
*** GonZo2000 has quit IRC11:42
*** GonZo2000 has joined #zuul11:42
*** GonZo2000 has quit IRC12:01
*** D3VIATION has joined #zuul12:25
*** rlandy has joined #zuul12:36
*** D3VIATION has quit IRC12:47
*** rlandy is now known as rlandy|rover12:55
*** pcaruana has quit IRC13:15
*** Wei_Liu has quit IRC13:52
*** jimi|ansible has quit IRC13:55
*** Wei_Liu has joined #zuul13:57
*** pcaruana has joined #zuul14:10
*** acozine1 has joined #zuul14:27
openstackgerritJames E. Blair proposed openstack-infra/zuul master: WIP Make file matchers overridable  https://review.openstack.org/57174514:37
*** pwhalen has quit IRC14:47
*** pwhalen has joined #zuul14:49
*** pwhalen has joined #zuul14:49
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Make file matchers overridable  https://review.openstack.org/57174516:00
*** harlowja has joined #zuul16:11
*** rlandy|rover is now known as rlandy|rover|brb16:14
*** gtema has quit IRC16:29
*** gtema has joined #zuul16:30
*** EmilienM is now known as EvilienM16:40
*** sshnaidm_pto has quit IRC16:44
corvusthat ^ looks to be ready now too.  that's a behavior change, so we'll probably want to call it version 3.116:49
*** electrofelix has quit IRC16:55
*** rlandy|rover|brb is now known as rlandy|rover16:55
*** electrofelix has joined #zuul16:55
*** gtema has quit IRC16:59
*** electrofelix has quit IRC17:01
*** harlowja has quit IRC17:22
*** hashar is now known as hasharAway17:45
*** myoung is now known as myoung|bbl17:47
*** jimi|ansible has joined #zuul17:59
openstackgerritMerged openstack-infra/zuul-jobs master: Handle -/_ ambiguity in package names  https://review.openstack.org/57100518:11
fungithat ^ got pretty thoroughly cross-tested and doesn't look like it should introduce any regressions, but just a heads up that it's touching some python logic in the zuul-jobs stdlib18:11
fungii _am_ around to help emergency revert if needed (i know, dicey changes late on a friday)18:12
clarkbI noted in the infra channel to that changes author that I think python pacakges are case insensitive as well so we may need to handle that corner case too18:13
*** harlowja has joined #zuul18:14
fungiyeah, shouldn't be hard to add since pkg_resources has a function for that (safe_name i think?)18:25
fungiand it's already importing from there18:25
*** openstackgerrit has quit IRC19:04
clarkbcorvus: stack starting at https://review.openstack.org/#/c/563194/5 has +2s fopr 5 or six changes. Did you want to go ahead and approve them? I'm going to keep reviewing towards the end of that stack19:10
clarkbbut first lunch19:12
*** toabctl has quit IRC19:12
*** toabctl has joined #zuul19:16
corvusclarkb: ah thanks, i'll go through those soon19:19
corvusoh, whoops, i said i was going to do the files/irrelevent-files change separately.  i'll split it out.  :)19:23
*** myoung|bbl is now known as myoung19:30
*** openstackgerrit has joined #zuul19:42
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Make file matchers overridable  https://review.openstack.org/57174519:42
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Make files and irrelevant-files matchers exclusive  https://review.openstack.org/57180419:42
*** jlvillal is now known as jlvacation19:43
clarkbcorvus: I'm reading https://review.openstack.org/#/c/571745/3/doc/source/user/config.rst and reconciling the removal of "matchers" against the commit message. I think at least part of the old matchers section is still accurate. You can't create a new variant with these matchers19:55
clarkbbranches was updated with that info but not files and irrelevant files? I'm making sure I'm not missing something specific to the new expected behavior19:56
corvusclarkb: well, i mostly removed that because the 3 matchers aren't the same anymore19:56
clarkbright branches is different than the other two. I guess my question boils down to should we document that files and its negation don't make a new vairant19:56
corvusoh, hrm, i think our vocabulary may be getting squirrely.  every job stanza is a variant.  it's just that files used to limit the applicability of that variant, but now it means do-or-don't-run-this-job19:59
clarkbgotcha19:59
clarkbcorvus: I guess that is why parent-override-job defined twice with two different files: entries is valid20:00
clarkb?20:01
corvusclarkb: exactly.  you used to be guaranteed only to get one of those.  now you'll always get both, and the second one wins.20:02
*** pcaruana has quit IRC20:03
corvus(essentially, that was a test of the ability to alter a job configuration by using a file variant, and that's the feature we lose with that change, so that's why that test changes)20:03
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Make file matchers overridable  https://review.openstack.org/57174520:15
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Make files and irrelevant-files matchers exclusive  https://review.openstack.org/57180420:15
*** hasharAway is now known as hashar20:18
clarkbquestion: is the migrate tool intended to be "supported"20:30
clarkbI see pabelanger is making updates to it.20:30
clarkb(I just worry about making changes to it as it doesn't have tests iirc)20:30
corvusthat's a good question; i was going to suggest we remove it.... but if it's still useful to someone and it's getting better(?) maybe we keep it?20:31
clarkbiirc we had a test that ran against infras config for it, but I don't think we still ahve that because infras config is transitioned20:31
corvusi feel like it ended up being the "openstack migration tool", so from that pov, if it works for whoever (pabelanger in this case) is making the change, it's probably no worse :)20:32
corvus(by the time we finished, it had a heap of assumptions built into it)20:32
clarkbya20:32
clarkbmaybe if we see continued use for it we should resurrect a test of some sort for it20:32
clarkbcorvus: I'm finding a lot of changes with many +2's in zuul. are there any I should hold off an approving if I review them and am happy with them?20:35
*** yolanda has quit IRC20:35
clarkbhttps://review.openstack.org/#/c/546698/ for example20:35
clarkb(then maybe next week we update the running version in openstack and cut a release with your behavior change too)20:35
corvusclarkb: i'm behind on reviews due to summit prep, so hard to say.  i'll go through them right now though if you want to leave any non-trival ones for me to +W.  i wouldn't mind catching up :)20:41
clarkbthe above one is potentially non trivial since it interacts with zuul. Then the github stack I linked earlier. I'm currently lookin at https://review.openstack.org/#/c/567226/1/zuul/merger/merger.py and trying to figure out if the git cleanup operation needs a lock20:43
clarkbI think the merger gearman tasks are serialized20:43
clarkbbut not sure if the executor mergers are20:44
clarkboh the "root" there is job specific so should be fine20:44
clarkbhrm no it defaults to merge_root = /var/lib/zuul/executor-git20:45
clarkbexecutor server has a merger_lock perfect20:49
clarkbtobiash: I left a comment on https://review.openstack.org/#/c/567227/1 about handling that general class of problem mroe robustly. Let me know what you think20:58
corvusclarkb: left a -1 on https://review.openstack.org/535680  - what do you think?21:03
clarkbcorvus: ++21:04
openstackgerritMerged openstack-infra/zuul master: Test more action modules  https://review.openstack.org/56700721:05
clarkbthe ansible 2.5 plunge is probably the other big thing we need to do soon21:08
openstackgerritMerged openstack-infra/zuul master: Add role information to task in zuul_json callback  https://review.openstack.org/55999821:08
* clarkb will defer thinking on that until next week21:08
openstackgerritMerged openstack-infra/zuul master: Support databases on other hosts during tests  https://review.openstack.org/56012721:08
*** hashar is now known as hasharZzz21:09
clarkbianws corvus: https://review.openstack.org/#/c/568195/4/zuul/web/__init__.py  I think that is superceded by the cherrypy work, can we go ahead and abandon that change?21:09
clarkb(I'm assuming we are moving forward with cherrypy based on it working at this point)21:09
corvusyeah, i think it's ready to go and we should do it21:09
openstackgerritMerged openstack-infra/zuul master: Only emit parent-change-enqueued if needed  https://review.openstack.org/56319421:10
openstackgerritMerged openstack-infra/zuul master: Gracefully handle corrupt local git repositories  https://review.openstack.org/56722621:10
*** acozine1 has quit IRC21:16
pabelangercorvus: clarkb: Can understand if we don't want to support it in zuul, I'm only pushing up changes it case anybody else wants to use it. But given the self imported deadline to get rdoproject on zuulv3 ASAP, it is a handy tool. But openstack specific for sure.21:16
clarkbpabelanger: I'm ok with it, mostly just worried about lack of testing if we do support it since the testing was openstack infra specific and now no longer exists aiui21:17
*** EvilienM has quit IRC21:18
*** EmilienM has joined #zuul21:18
*** EmilienM has joined #zuul21:18
*** EmilienM is now known as EvilienM21:19
corvusi'm +2 on the migrate changes; clarkb are you going to review them?21:22
clarkbcorvus: ya I'll review them21:22
openstackgerritMerged openstack-infra/zuul-website master: Add a "zuul: gated" status badge  https://review.openstack.org/56897521:23
pabelangerclarkb: agree about testing, I can see about reverting https://git.zuul-ci.org/cgit/zuul/commit/tools?id=badc54be7eef6d0b244e4cb3881f651f64100c10 to see how testing works21:24
pabelangeror work to iterate on it21:25
corvusSpamapS: https://review.openstack.org/571342  should unblock your github status work21:25
corvusclarkb: looks like the +W on cherrypy is yours :)21:26
clarkbcorvus: I'll review that after the migrate cahnges I guess21:27
SpamapSoh neat!21:35
openstackgerritMerged openstack-infra/zuul master: Remove unnecessary injections  https://review.openstack.org/57134121:42
openstackgerritMerged openstack-infra/zuul master: Support merged as requirement in github driver  https://review.openstack.org/56848821:42
openstackgerritMerged openstack-infra/zuul master: Allow templates to be optional for zuul-migrate  https://review.openstack.org/57097921:42
clarkbthe two prereq changes to cherrypy have been approved21:46
openstackgerritMerged openstack-infra/zuul master: Allow for projects only names in zuul-migrate  https://review.openstack.org/57098021:47
openstackgerritMerged openstack-infra/zuul master: Add the ability for a user to skip macros  https://review.openstack.org/57128721:47
clarkbcorvus: what is the purpose of the save params tool?22:07
clarkbthat a rest api thing because the api is path based on query string based?22:07
corvusclarkb: yeah, if you use the routes handler, it will pass the path params to the function, but cherrypy will natively do the same thing with get/post params.  i didn't want someone to be able to do '/tenant/foo/something?tenant=bar' and override the path param with the query param.22:12
corvusclarkb: i think if we drop routes and go with traversal along with some special handler, we can avoid that in the future.  but i wanted to use routes for the first change at least to keep the delta smaller.22:13
openstackgerritMerged openstack-infra/zuul master: Move SQL web handler to driver  https://review.openstack.org/56802822:15
clarkbcorvus: and cherrypy's engine handles each request in its own execution context so we don't ahve to worry about concurrency ourselves right?22:16
openstackgerritMerged openstack-infra/zuul master: Add never_capture test decorator  https://review.openstack.org/56951522:18
corvusclarkb: yeah, the default server (which we're using) uses thread pools.  it's called cheroot: https://github.com/cherrypy/cheroot22:18
corvuscherrypy can be used with other wsgi environments too22:18
clarkbhttps://chromium.googlesource.com/external/googleappengine/python/+/1e5241cb0e48d0cb192fa944f2e218c4a18b244e/lib/cherrypy/cherrypy/__init__.py?autodive=0%2F#17122:22
corvusclarkb: probably the only thing to be aware of there is that cherrypy uses thread-local objects for requests.  so when a method access 'cherrypy.request' it's accessing "the current request being handled by this thread"22:22
clarkb:) that was what I was just looking aty22:22
corvusclarkb: that's in contrast to other systems where the request is passed in to handlers as an argument22:22
corvussame thing for cherrypy.response22:22
clarkbright22:22
clarkbits "global" but not really22:22
clarkbI guess that makes the code look nice22:22
corvusyeah, i think the idea is that the function arguments and return values are then literally just the input/output from the server-side methods.22:24
clarkbcorvus: https://review.openstack.org/#/c/568335/20 question on that. My reading of the code is that we are probvably ok but you may be more familiar with it22:35
corvusclarkb: we should be fine because in web/__init__.py we use the incremental decoder to make sure that we only send valid utf8 chunks over the websocket.  so the ws client should only receive valid chunks.22:38
*** rlandy|rover has quit IRC22:39
corvusclarkb: i believe that's the equivalent code to what was there before (ie, aiohttp's client was giving us unicode strings, but ws4py is giving us bytes)22:39
clarkbcool. I think if recieve_message could get half a message we would have to worry about it but it claims to only be called on completely messages at least22:40
clarkbcorvus: one last question before I approve. the info parameter to zuulweb seems mostly unused?22:43
clarkbcorvus: state to clean out in a followup?22:43
corvusshould still be used -- ZuulWebAPI.info() uses it22:44
corvus(via a reference it holds to the zuulweb object)22:44
clarkbcorvus: ya that part is fine btu we also pass in info as a distinct reference then don't appear to use it22:45
clarkber to the connection objects not zuulweb22:45
clarkbsorry half of it is unused the other half is used22:45
clarkbcorvus: https://review.openstack.org/#/c/567959/23/zuul/web/__init__.py line 34322:46
clarkbI've gone ahead and approved the cherrypy change22:48
clarkbI think ^ can be cleaned up in a followup if it does need cleaning up22:48
* clarkb writes that patch may be easiest to explain that way22:51
*** D3VIATION has joined #zuul22:53
openstackgerritClark Boylan proposed openstack-infra/zuul master: Cleanup unused connection.getWebController info param  https://review.openstack.org/57190122:53
clarkbcorvus: ^ that22:53
corvusclarkb: yeah, i think you hit something with rough edges.  we used to set capabilities.job_history inside of the of the sql driver if there was a sql connection.  but i dropped that, and there's no tests for it, so i didn't notice it.22:54
corvusi don't think anything actually used it yet either22:54
clarkbas you point out it is accessible from the also passed in ZuulWeb object so I don't think it is necessary even if you want the data later22:54
corvusclarkb: yeah. i think your change is a good next step.  i don't think the job_history is a big issue; i'm not even sure the old code was quite correct.  if/when we want to make the web interface not have a builds tab if you don't have the sql reporter, we'll need to fix it.  but it should be easy to do.  no more difficult than under aiohttp.22:58
corvusand when we do, we'll add a test :)22:59
corvusbecause, as i've helpfully demonstrated, if it's not tested, i will break it22:59
clarkbwe need that put on a stone somewhere23:00
openstackgerritMerged openstack-infra/zuul master: Replace use of aiohttp with cherrypy  https://review.openstack.org/56795923:02
openstackgerritMerged openstack-infra/zuul master: Convert streaming unit test to ws4py and remove aiohttp  https://review.openstack.org/56833523:02
clarkbcorvus: you missed https://review.openstack.org/#/c/563297/7 and its children. I wasn't going to approve them myself as I'm trying to avoid needing to review them :P23:04
clarkb(you have alredy +2'd but not approved)23:05
openstackgerritMerged openstack-infra/zuul master: Use iterate timeout in streaming tests  https://review.openstack.org/57149823:10
openstackgerritMerged openstack-infra/zuul master: Use ZuulWebFixture in tests  https://review.openstack.org/57149923:10
*** harlowja has quit IRC23:18
fungijust a general heads up since people running zuulv3/nodepool are almost certain to have a zk cluster... CVE-2018-8012 means that you should really, really be using some sort of firewall to restrict zk communication source addresses23:19
fungihttps://zookeeper.apache.org/security.html#CVE-2018-801223:20
corvusclarkb: thx23:22
*** hasharZzz has quit IRC23:27
openstackgerritMerged openstack-infra/zuul master: Extend github testing using app auth  https://review.openstack.org/56329723:42
openstackgerritMerged openstack-infra/zuul master: Test parent-change-enqueued with github  https://review.openstack.org/56324223:42
*** jimi|ansible has quit IRC23:47

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!