19:00:56 <jillr> #startmeeting ansible core public irc meeting
19:00:56 <zodbot> Meeting started Tue Aug 13 19:00:56 2019 UTC.
19:00:56 <zodbot> This meeting is logged and archived in a public location.
19:00:56 <zodbot> The chair is jillr. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:00:56 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
19:00:56 <zodbot> The meeting name has been set to 'ansible_core_public_irc_meeting'
19:01:54 <jillr> felixfontein: has #59060 been discussed?  noticed it wasn't checked off but's been on the list for a while
19:02:21 * jillr skimming thru logs
19:03:00 <jillr> looks like it was, just want to make sure we dont pass you by
19:03:12 <jillr> #topic open floor
19:03:52 <resmo> having two open PRs, which I like to discuss
19:04:03 <jillr> sure thing resmo
19:04:07 <resmo> https://github.com/ansible/ansible/pull/56549
19:04:14 <jillr> #topic https://github.com/ansible/ansible/pull/56549
19:06:00 <resmo> There seems to be uses cases for "no updating the cache" even the dependency for the apt module is not installed.
19:06:09 <felixfontein> jillr: I think it was not discussed, or I don't remember :)
19:07:30 <resmo> any comments on that #56549?
19:07:34 <felixfontein> the last meeting was very short, and we only discussed #60178 I think
19:07:43 <sivel> Oh, hey, was chasing a rabit
19:07:51 <sivel> not a real one
19:08:06 <felixfontein> resmo: still looking
19:08:12 * jillr still reading PR...
19:09:04 <sivel> I remember talking about this PR at one point, and we weren't convinced whether it was the right option or not
19:09:23 <sivel> since python-apt is a necessity, if it is missing, we need to ensure it installs
19:09:24 <felixfontein> brb
19:09:32 <sivel> but I don't remember much of that conversation we had
19:10:02 <sdoran> I believe the conclusion is you can do this via `loop` and it was not necessary to add it to the module.
19:10:20 <resmo> sdoran, imho that was another PR
19:10:22 <sivel> I think this is a different discussion
19:10:23 <sdoran> But resmo had a counter example to that.
19:10:32 <sivel> https://github.com/ansible/ansible/pull/56549/files
19:10:32 <sdoran> Ok, I'm getting confused then.
19:10:38 <jillr> it looks like this handles installing python-apt, while also skipping cache update if specified
19:10:38 <cyberpear> 56549 lgtm
19:10:39 <felixfontein> sivel: it is still installing the dependency, it is just not running 'apt-get update' first
19:10:47 <sdoran> Too many apt caching related things. :)
19:10:47 <jillr> which seems to make sense to me
19:10:51 <sivel> felixfontein: which may fail, if we don't have a cache
19:11:32 <sivel> sdoran: this is about us running apt-get update before trying to do apt-get install python-apt, regardless of the update_cache setting
19:11:46 <jillr> sivel: so we can catch that exception then and fail
19:12:00 <felixfontein> sivel: true, but then the module will fail and the user can do something about it
19:12:28 <sivel> potentially, I'm just trying to state things that I remember from a previous discussion
19:12:51 <sivel> also, we are lacking core quorum today, with only 3 of us, there can't be a decision
19:14:30 <jillr> resmo: can you put these PRs on the agenda for Thur? https://github.com/ansible/community/issues/482
19:14:41 <resmo> sure
19:14:44 <jillr> thanks
19:14:59 <jillr> I cant make Thu but I get the use case and I'm generally +1
19:15:33 <jillr> unless there's some new, substantive counter argument
19:15:48 <resmo> ok
19:16:00 <jillr> resmo: would you like us to take a look at the 2nd PR today even though we cant get quorum?
19:16:19 <resmo> yes, https://github.com/ansible/ansible/pull/57266
19:16:27 <jillr> #topic https://github.com/ansible/ansible/pull/57266
19:17:16 <sivel> should this have a exponential backoff?
19:17:30 <sivel> most of our retry logic pauses for some increasing interval per failure
19:17:47 <jillr> sivel: +1
19:17:47 <resmo> sivel, good point. I guess that would make sense.
19:18:04 <resmo> (and apt module should have it too then)
19:18:14 <cyberpear> also lgtm, though backoff also sounds good
19:19:33 <resmo> sivel, any pointers where we already have a backoff logic implemented?
19:19:50 <felixfontein> +1 for also doing that in apt
19:19:50 <jillr> resmo: several of the aws modules
19:19:57 <sdoran> The `reboot` plugin.
19:20:01 <resmo> ok. thx
19:20:23 <resmo> I'll update the pr and also create a similar logic in apt
19:20:33 <jillr> awesome, thanks resmo
19:20:43 <jillr> #topic https://github.com/ansible/ansible/pull/59060
19:21:12 <jillr> felixfontein: aiui you said we havent discussed this one yet right?
19:21:26 <sdoran> resmo: https://github.com/ansible/ansible/blob/60f84836031f51af1b0807476e13a0a6313b7082/lib/ansible/plugins/action/reboot.py#L262
19:21:48 <felixfontein> I originally wanted to delay this until #58646 merged, but that doesn't seem to be happening, so I'm proposing #59060
19:21:53 <felixfontein> jillr: yes, it hasn't been discussed
19:21:56 <cyberpear> (hooray for permalink!)
19:22:02 <resmo> sdoran, ack
19:22:28 <felixfontein> #59060 adds some extra validations related to option name aliases
19:22:52 <felixfontein> in some cases, modules document an alias of an option name, which prevents proper validation of the option's docs to happen
19:23:35 <sivel> Isn't 390       argument in argument_spec is not documented a duplicate? of 322?
19:23:44 <felixfontein> I also added some basic sanity checks on aliases which I think should be uncontroversial
19:23:47 <sivel> > 322       argument is listed in the argument_spec, but not documented in the module
19:25:44 <felixfontein> ah, good point
19:25:58 <felixfontein> the current checks for 322 and 323 do not consider aliases
19:26:58 <sivel> might be worth consolidating them
19:27:24 <felixfontein> yes, definitely
19:28:56 <felixfontein> but are the other tests in general ok? or is there something which should be handled more strictly, or less strictly, or not error'ed at at all?
19:30:06 <sivel> Is 388 saying that one option specifies it mulitiple times, or that it is an alias of multiple arguments?
19:30:21 <sivel> If it's just that aliases=['foo', 'foo'] I'd say is that important?
19:30:47 <felixfontein> it's aliases=['foo', 'foo']
19:30:52 <sivel> 388 and 389 seem at first look to be unnecessary really
19:31:01 <sivel> a nitpick, as opposed to an actual problem
19:31:16 <felixfontein> (I don't think I'm currently checking for same alias being used for multiple options)
19:32:07 <felixfontein> they're usually left-overs of copy'n'pasting, and could be potential future errors
19:33:09 <felixfontein> I can remove them if you prefer
19:33:45 <sivel> I mean you took the time to write them, they don't hurt anything, I was just questioning their usefulness
19:35:35 * cyberpear hoping to get some eyes on https://github.com/ansible/ansible/pull/59958 if there's time at the end
19:36:39 <jillr> so I think the gist is, we're not opposed to #59060 but please deduplicate 322/390, and we should discuss it further when we have more core team around?
19:36:59 <felixfontein> sounds good to me
19:37:01 <sivel> sounds about right
19:37:09 <jillr> cool, thanks felixfontein
19:37:14 <felixfontein> then let's do that :) thanks everyone!
19:37:19 <jillr> #topic https://github.com/ansible/ansible/pull/59958
19:37:33 <felixfontein> I also have one more thing for later
19:37:38 <jillr> ack
19:39:37 <sivel> I believe the concern with 59958 was accidental exposure of info. I wasn't the one with the concern, so I don't fully know what we mentioned needed further investigation
19:40:17 <sivel> but _clean_results basically only helps clean up debug tasks
19:41:13 <sivel> I wonder, if to solve both concerns we left _clean_results where it is, but did a: changed = result._result.get('changed', False)
19:41:17 * jillr reading previous logs to find the concern
19:41:18 <sivel> before it, and then use that var
19:41:27 <sivel> jillr: it was in the triage meeting
19:41:38 <sivel> so no logs really, it was just people talking
19:42:18 <felixfontein> you don't do voice recording + speech2text on it? ;)
19:42:43 <sivel> Was thinking something like this change instead: http://dpaste.com/333VZNC
19:42:57 <jillr> sivel: ahk, thx
19:44:30 <felixfontein> that should also solve it, I guess
19:46:35 <jillr> cyberpear: any questions on^?
19:48:23 * jillr will give them another minute
19:49:31 <jillr> felixfontein: what else do you have?
19:50:34 <felixfontein> I started with an issue template to use for the _facts modules which return ansible_facts but should not; and I'd like a quick look of you all on it: https://gist.github.com/felixfontein/99276133a74149a27103a9c4fd5431bb
19:50:48 <jillr> #topic https://gist.github.com/felixfontein/99276133a74149a27103a9c4fd5431bb
19:51:13 <felixfontein> I took azure as an example (even though they are probably solving this by themselves), but I will use this template ~27 more times, so it would be nice if I don't repeat some crap 28 times ;)
19:52:08 <felixfontein> my train is arriving in a minute, so I'll be afk for ~10 minutes
19:52:20 <felixfontein> feel free to write anything that comes into your mind here or as a comment to the gist :)
19:52:25 <felixfontein> thanks everyone in advance! :)
19:52:35 <jillr> felixfontein: maybe dont want a hard value of 'Ansible 2.13' in the template unless you anticipate closing these all out in the next couple weeks
19:52:44 <jillr> cya felixfontein
19:54:05 <jillr> looks generally reasonable to me, will give it a more thorough read and leave any comments on the gist
19:54:23 <jillr> #topic open floor
19:57:04 <jillr> #endmeeting