19:00:01 #startmeeting Public Core Meeting 19:00:01 Meeting started Tue Oct 4 19:00:01 2016 UTC. The chair is jtanner. Information about MeetBot at http://wiki.debian.org/MeetBot. 19:00:01 Useful Commands: #action #agreed #halp #info #idea #link #topic. 19:00:01 The meeting name has been set to 'public_core_meeting' 19:00:15 #chair resmo dminca1 19:00:15 Current chairs: dminca1 jtanner resmo 19:00:46 Greetings 19:00:47 #chair abadger1999 alikins bcoca gundalow gundalow jimi|ansible mattclay mordred newtMcKerr nirik rbergeron Shrews 19:00:47 Current chairs: Shrews abadger1999 alikins bcoca dminca1 gundalow jimi|ansible jtanner mattclay mordred newtMcKerr nirik rbergeron resmo 19:01:08 greetings 19:01:14 anyone else around? 19:01:54 hi 19:01:55 #topic https://github.com/ansible/community/issues/127#issuecomment-250963384 19:02:31 ok 19:02:38 @resmo 19:02:44 resmo: i think you are discussing those handler issues in #ansible-devel, right? 19:02:52 right 19:03:08 yo 19:03:14 i told jimi|ansible that you'd be talking about handlers, so i'll grab him if he's needed 19:03:21 and not responding 19:03:30 yo 19:03:54 ^ does not mean i'm actively paying attention at all times though :) 19:04:12 resmo: so do you have some more debugging to do on your side before we can do something? 19:04:38 ok, first I would like to know the opinion about this PR 19:04:38 * mordred waves 19:05:17 https://github.com/ansible/ansible/pull/17847 19:05:17 resmo: 17070? 19:05:33 imho we should force handler to have a name. 19:05:33 #link https://github.com/ansible/ansible/pull/17847 19:05:50 not sure if this is the best implementation... 19:06:00 resmo: doesnt that go against the new directive? 19:06:16 #topic https://github.com/ansible/ansible/pull/17847 19:06:20 if handler is defined using 'subscribe', no need fo rname 19:06:23 bcoca: you are right but while I tested, I got a problem https://github.com/ansible/ansible/issues/17846 19:06:40 if they don't have a name. the module "name" is used 19:07:15 yes, but that should not trigger their execution, if that is what is happening, its a diff bug 19:07:18 if you have 2 handler using same module, you have identical names which results in only exectues one 19:07:19 requriing name is not right fix 19:07:45 how do you notify an unnamed handler? 19:07:46 I am ok with a different approach 19:07:52 name: handler <= if user does that but relies on listen .... 19:08:20 jtanner: you can find an example here https://github.com/ansible/ansible/issues/17846 19:08:26 it the issue I created 19:08:46 i saw ... that's why i ask 19:09:05 in the sense that i agree having an unnamed handler seems odd 19:09:07 problem there is that we did not implement 'listen' w/o disabling teh 'unique name per handler restriction', which should be 'on call' but not on definition 19:09:29 jtanner: there is always a name, that is not the issue, the problem is duplicate names 19:09:38 right 19:09:38 which 'non name' does by default when the action is the same 19:09:44 "always", meaning it's "name" if not defined? 19:10:03 since we introduced 'listen', handlers are not required to be uniquely named 19:10:14 name is always defined 19:10:26 as tasks will create a name when user does not supply one 19:10:37 using 'action' and depending on config 'param's to create it 19:10:46 got it 19:10:56 https://github.com/ansible/ansible/blob/devel/lib/ansible/playbook/task.py#L120 19:11:14 so if using 'name' to trigger the handler, this works as expected, but using 'listen' it should allow for dupe names 19:11:20 resmo, i understand the problem 19:11:36 we keep track of notified listeners in a dict, so if more than one has no name it will cause a problem 19:11:48 and as bcoca pointed out if users use identical names the latter will be overwritten as well 19:11:52 dupe names, no name is fine as long as the generated name does not match 19:12:11 or not taken to be more precise 19:12:11 perhaps we should track based on the uuid field instead, instead of the name 19:12:26 jimi|ansible: only for the 'listen' ones 19:12:38 the 'old handler invocation by name' shoudl still work like that 19:12:38 #note issue is related to dict store of handlers which does not allow for duplicate names 19:12:51 well, that'd fix both problems, in the case of dupe names it would find the first and use its uuid instead of the later 19:13:01 which is how it behaves today 19:13:24 * MichaelBaydoun is lurking 19:13:37 #chair MichaelBaydoun 19:13:37 Current chairs: MichaelBaydoun Shrews abadger1999 alikins bcoca dminca1 gundalow jimi|ansible jtanner mattclay mordred newtMcKerr nirik rbergeron resmo 19:13:39 well, 2 diff features, listen was supposed to allow for dupe names as main reason was for handlers to be included from diff sources w/o regards of existing handlers 19:13:55 old handler name, yes, should continue as is, but listen should be more flexible 19:14:13 that wasn't the intention with listen 19:14:28 it may have been kind of a side effect in some situations but it wasn't the primary motivation 19:14:36 i would argue if we still force unique names, everyone adding their own 'apache config handler' will name them the same 19:14:55 well, not the name uniqueness, but it is in spirit of the feature, 'arbitrary handlers' 19:16:09 if we are fine with requireing unique names for handlers, i would say this is not a bug, works as expected 19:16:10 is the handler dict scoped to the entire play or is their a dict for roles too? 19:16:19 jtanner: roles are in play 19:16:27 so its always play scoped 19:16:38 ^ the handler list 19:16:42 yeah per-play, can't notify across plays 19:16:44 but in the context of unnamed handlers, would the notification go to just the handler in the role? 19:16:58 from a task inside a role 19:17:00 no, handlers get imported onto play handler list 19:17:05 k 19:17:07 last one defined wth same name 'wins' 19:17:27 jimi|ansible: if we are fine with listen having the same restriction, this is not a bug then 19:17:55 i woudl argue we should not apply that limitation, but not going to make a fuss over it 19:18:02 fix is not easy 19:18:11 2 diff lists of handlers 19:18:48 the lists of handlers is not the problem, it's the notified dict in TQM 19:18:52 that's easy to fix 19:19:00 let me reprhase, 1 dict of handlers 'name is key', 1 dict of 'listen' with value being list of handlers to execute 19:19:25 ^ can share same dict in tqm, probably want to give diff value? 19:19:33 to indicate name vs listen? 19:19:33 if i remember right too, we actually put the handler object in the list, not just the name 19:19:45 and there's a separate dict for listeners yes 19:19:54 then not sure why this happens 19:20:03 unless we are forcing unique handlers on load? 19:20:04 because it's still matching on the name 19:20:15 why? if listeners have their own ref? 19:20:25 and the name is "set_fact" for resmos example for both when `name: ` is not specified 19:20:38 which woudl be expected on notify by name 19:20:47 and is not a bug in that case 19:21:00 its only a bug if we consider that handlres using listen do not require unique names 19:21:20 bcoca: I can understand your args 19:21:38 which sounds liek we can go either way and code is almost there, but we need to decide on standard and then make it so 19:21:44 ok lets move on, just assign these to me if they're not already and i'll get to a definitive answer (i'm doing this from memory right now) 19:21:53 ok, next one 19:22:06 this is the issue https://github.com/ansible/ansible/issues/17855 19:22:07 next issue in agenda? 19:22:12 or in resmo's comment? 19:22:38 #topic https://github.com/ansible/ansible/issues/17855 19:22:45 and this is the fix https://github.com/ansible/ansible/pull/17856 19:23:10 if you notify a value used for listen and as handler name, listen would be taken 19:23:18 the first implementation used the handler name 19:23:34 and ignored listen 19:23:56 should we merge them or keep as separate channels? 19:23:58 (there is also another fix in this PR, which I come to it later) 19:24:36 bcoca: channels? 19:24:38 ah, i see, we changed it from initial implementatio.n which used name first 19:24:45 notify by name vs notify/listen 19:24:49 right 19:24:58 but yes, makes sense 19:25:06 name => listen 19:25:09 right 19:25:40 but does that mean that if name matches, no listen is possible? 19:26:17 it means if you named your listen the same as a handlers name, listen is ignored 19:26:32 is that what we want? 19:26:56 yes, I think so it makes sense 19:26:58 if you have pay with 'listen' and add a role in which my handler happens to match by name, i invalidate your existing handlers? 19:27:17 yeah it's possible i just screwed that up when killing the results worker 19:27:18 ^ not that you should add roles w/o inspecting them ... 19:27:35 i'm fine with order, just thinking about 'why not both?'' 19:28:00 good question, 19:28:04 might be confusing? 19:28:14 but yeah i think both might be good 19:28:29 change looks good, we can sovle the 'just run both issue later' 19:28:41 ok, I'll change it 19:28:55 #note named handler and listen order of precedence changed 19:29:04 https://github.com/ansible/ansible/issues/17848 <= this, i thought we were making it configurable, error by default, but warning if config allowed 19:29:15 jtanner: more like restored 19:30:04 bcoca: this is also in the PR https://github.com/ansible/ansible/pull/17856/files#diff-b893b8b8319cd4518208dd4fe53f3d7bR426 19:30:17 resmo: separate PRs? its a bit confusing to have multiple fixes in 19:30:23 agree 19:30:41 also that is a hardcoded change, we agreed on 'config dependant' 19:30:59 resmo: we decided to go back to raising an error when handlers were not found 19:31:02 ok, so I'll remove it from the PR 19:31:02 so if not handler_found and C.MISSING_HANDLER_IS_ERROR: .... 19:31:08 that was the 1.9.x behavior 19:31:20 yes, error by default, but we should let user turn into warning 19:31:24 i am fine with a config option 19:31:46 would someone take care for implementation? 19:32:18 i'll do 19:32:37 great, thx 19:32:43 * resmo is on vacation soon... 19:33:01 that is it from me :) 19:33:05 so what's the result here? 19:33:29 #agreed bcoca will add an option to switch between errors/warnings for missing handlers upon notification 19:33:45 ok, next agenda item? 19:33:49 #action bcoca will add an option to switch between errors/warnings for missing handlers upon notification 19:34:11 hrm, no bot feedback on those? 19:34:18 no permissions 19:34:18 guess not 19:34:23 #topic https://github.com/ansible/ansible-modules-core/issues/4613 19:34:27 dminca1: ^ 19:34:30 i'm chaired, should be allowed 19:34:32 thank you 19:34:40 at first glance, that seems like a python lib issue 19:34:49 I think the cloud/digitalocean module needs to be updated with new DigitalOcean API DigitalOcean v2 since the end-of-life for v1 has been declared on November 9, 2015 (as stated in https://developers.digitalocean.com/documentation/changelog/ ) 19:35:03 Since it's an important module (like all others), I think it requires utmost attention, because it's the era of Cloud :) and tbh, not only myself was disappointed to see that after writing 1h code to test digitalocean droplet creation, I received a stderr with `NameError: name 'DoError' is not defined`... (no offence to anyone) 19:35:09 jimi|ansible: yeah the only thing with bot feedback is #undo and #topic (because that change the channel topic) 19:35:21 I've also noticed on the official DigitalOcean dev website that dopy is no longer maintained ( https://developers.digitalocean.com/libraries/# ) and the link to dopy returns 404 ( https://www.digitalocean.com/community/projects/dopy ). 19:35:41 dminca1: I think I can come up with a fix for that... Can you test it? 19:36:16 abadger1999 aye 19:36:19 will do 19:36:28 Cool. I'll submit a PR. 19:36:58 #action abadger1999 to submit bugfix for dminca1 to test 19:37:07 kgadek suggested to install `six` 19:37:11 dminca1: For dopy being deprecated... I don't think any of the core devs use digital ocean so we can't do big changes like porting to another library. 19:37:14 but that's not a way to fix 19:37:29 we would rely on community for porting 19:37:48 dminca1: If you can find a maintainer for the modules we might be able to expedite them becoming a comaintainer of hte module (if the modules are uncared for now) 19:38:19 abadger1999 woah that's going to be difficult, but will try 19:38:27 :-) 19:38:29 maybe someone from #ansible 19:38:36 alukovenko's github activity is pretty low 19:38:48 jus something to bear in mind. 19:39:27 I don't get it why this module's not so popular, I think most of people make droplets on digital ocean, but don't do'it via Ansible (some of them) 19:39:46 when I did a search on github for Ansible Digitalocean I found quite some projects tbh 19:39:49 would be nice for d.o. to maybe own their modules 19:40:05 why jtanner aws owns them ? 19:40:24 i favor vendor participation in module dev 19:40:36 ahh yes, me too 19:40:41 testing is expensive for us 19:40:54 testing can be done easily via Docker :D 19:40:59 for DO? 19:41:08 no lol 19:41:13 is there a DO api mock container? 19:41:29 well I'm willing to donate 19:41:34 that's where the expense comes from ... having an endpoint to run integration against 19:42:05 I just won 20 bucks for registering on docker.cloud 19:42:33 how much cash do you guys need ? 19:43:14 https://github.com/ansible/ansible/pull/17902 <= resmo, jimi|ansible 19:43:16 ok, so will need to look for co-maintainer 19:44:11 dminca1: it's a multi-faceted expense. cost of instances, cost of people to maintain account * every cloud we have modules for 19:44:32 we don't even have aws in CI yet 19:44:32 https://github.com/ansible/ansible/pull/17902/files#diff-b893b8b8319cd4518208dd4fe53f3d7bR29 19:44:39 jtanner yes I get it :) 19:44:41 ^ bcoca not a fan of doing it that way 19:44:50 we always import as C and do C.WHATEVER 19:45:05 i can fix that, but seems like huge import for one constant 19:45:19 #topic open-floor 19:45:36 what's open-floor ? 19:45:36 anyone have other topics to discuss? 19:45:40 ah ok 19:45:57 !#agreed 19:45:57 jimi|ansible: fixed 19:46:30 dminca1: i could swear we did update do to use v2 .... 19:46:31 bcoca: can't we now just remove the else here as the code should also run if listen are found https://github.com/ansible/ansible/pull/17902/files#diff-b893b8b8319cd4518208dd4fe53f3d7bL407 19:46:36 or was that just inventory script? 19:46:39 dminca1: https://github.com/ansible/ansible-modules-core/pull/5154 19:46:52 or do you make a separate commit for it 19:46:55 resmo: that was my point, but trying to keep 1 change per PR 19:47:03 sure, go ahead 19:47:15 ^ jimi|ansible was going to decide on that 19:47:24 we should also document the behaviour 19:47:25 yes, all fine 19:47:38 k 19:47:43 bcoca: I think we updated to v but now they're deprecating the library? 19:47:47 change seems simple, im fine either way, think 'both is better' but not going to press it 19:47:56 thank you abadger1999, will test tomorrow morning 19:48:02 Cool. 19:48:09 i thought we agreed both was fine, but do a separate PR please 19:48:10 abadger1999: yeah, that requries full rewrite and no one has stepped forward 19:48:14 dminca1: Feel free to ping me in #ansible-devel if I forget about the ticket. 19:48:27 jimi|ansible: ok, so then just unique names to decide 19:48:39 dminca1: I get tons of email but I can still keep up with IRC ;-) 19:48:48 so, nobody has chimed in with topics .... so ... 19:48:59 going to end meeting in 10? 19:49:21 abadger1999 will do 19:50:24 #endmeeting