19:01:47 <jillr> #startmeeting ansible core irc public meeting
19:01:47 <zodbot> Meeting started Tue Sep 17 19:01:47 2019 UTC.
19:01:47 <zodbot> This meeting is logged and archived in a public location.
19:01:47 <zodbot> The chair is jillr. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:01:47 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
19:01:47 <zodbot> The meeting name has been set to 'ansible_core_irc_public_meeting'
19:02:16 <jillr> #topic https://github.com/ansible/ansible/pull/61659
19:02:29 <jillr> hey felixfontein, how's it going
19:02:54 <felixfontein> good, lot's of stuff to do, but not as bad as last week ;) how about you?
19:03:20 <jillr> just coming off another meeting so scrambling to catch up for this one  :)
19:03:27 <jillr> fest is next week already!
19:03:35 <felixfontein> hehe :) there doesn't seem to be too much stuff for today's agenda
19:03:48 <jillr> I know you had 2 things from a bit back
19:03:58 <jillr> I'm just now reading them though
19:04:03 <felixfontein> yep, also noticed that... the contributor summit is only in 6 days!
19:04:27 <felixfontein> I mainly put them on the agenda to get some feedback if/how these things should be done
19:04:32 <jillr> so yeah, just a quiet week here in ansible-land ;)
19:05:08 <felixfontein> the PR in $topic is I think useful, since it handles a thing several modules do in a nicer way
19:05:11 <felixfontein> yep :)
19:05:30 <agaffney> at first glance, that PR looks reasonable
19:07:09 <felixfontein> a potential change in behavior is that load_file_common_arguments() does some things with the secontext of the existing file, hence using it with a different file could maybe result in a different result than taking the result and changing the path
19:08:10 <jtanner> hi
19:08:23 <jtanner> goneri and i are on bluejeans, so we'll be spotty here
19:09:21 <felixfontein> I'm not really familiar with selinux / selinux contexts and so have no idea whether that's a bad change or not :)
19:10:20 <jillr> ok so on a read of the PR it seems to make sense, it would be good though to call out the selinux item for discussion in the PR itself in case we dont get anyone today who knows that area really well
19:10:41 * nitzmahone lurks as well
19:13:52 <jillr> anyone else have any thoughts on 61659?  beyond one of the reviewers needs to be able to comment on selinux impact?
19:14:21 <nitzmahone> Also looks reasonable to me at a glance
19:14:34 <bcoca> not sure why you want to change the path
19:14:45 <nitzmahone> IIRC copy and a few other things hack it
19:15:02 <felixfontein> bcoca, because the module wants to write to two files (like args['path'] and args['path'] + '.pub')
19:15:28 <bcoca> ah, the original assumes path option as normally we only write one
19:15:46 <bcoca> other paths supplied are normally a source
19:16:03 <nitzmahone> So this theoretically makes such tweaks less error-prone
19:16:11 <bcoca> +1
19:16:27 <felixfontein> assuming that the selinux context details don't screw something up :)
19:16:37 <bcoca> depends, those can be tricky
19:16:50 <felixfontein> for the users which modified module.params['path'] before calling the function, it shouldn't change the behavior
19:16:56 <bcoca> as long as both files use same context .. .but  '.pub' ? is this public and private key?
19:17:11 <felixfontein> for the ones which patched the result, it might be
19:17:24 <bcoca> you might requrie diff contexts for those as one is meant to be only accessed by owner and the other is freely readable
19:17:33 <felixfontein> that's from the openssh_keypair module, it writes args['path'] and args['path'] + '.pub'
19:18:10 <bcoca> i would check selinux policies,  i don't know by heart, but i suspect you need diff context tags
19:19:23 <bcoca> so change to base function looks ok, but crypto module might require a diff approach
19:20:05 <felixfontein> yep, but that's a different problem anyway :) the behavior of that module is currently not optimal
19:24:09 <jillr> ok so, please put some thought into selinux but otherwise we like this change, lg.
19:24:26 <jillr> #topic https://github.com/ansible/ansible/issues/61964
19:24:36 <jillr> felixfontein: you're still up!
19:24:53 <felixfontein> kind of hard since I really don't use it, and have no idea whether what the effect would be. I can do some reading, but chances are I miss something :)
19:24:56 <felixfontein> yay!
19:25:05 <felixfontein> that one is more a question on how this bug should be fixed
19:25:45 <bcoca> is  this recent change from old 'startswith'?
19:25:45 <felixfontein> i.e. error out, or be more lenient in some cases (I would only allow trailing comma, but not double inbetween comma or a string consisting of one comma)
19:26:22 <felixfontein> oh, I just saw sdoran created a PR for this
19:26:35 <bcoca> 284dafe476870a924e21c7d060f5abf5742ebaa9 <= @sivel, this caused it
19:26:47 <bcoca> better performance .. but it breaks
19:27:29 <felixfontein> https://github.com/ansible/ansible/pull/62442 here's the PR
19:27:30 <bcoca> 2ebc4e1e7eda61f133d9425872e2a858bae92ec4 <= but didnt this fix it?
19:28:09 <bcoca> i c, what is there to discuss?
19:28:25 <bcoca> hmm, we strip x2
19:28:42 <bcoca> i would switch to for loop and pass resulting list
19:28:48 <felixfontein> what should `--limit ,` do?
19:28:56 <bcoca> really, nothing
19:29:07 <bcoca> its limit to these empty lists
19:29:08 <felixfontein> so equivalent to `--limit ''`?
19:29:14 <bcoca> yep
19:29:47 <felixfontein> `--limit ''` doesn't do any limiting for me, i.e. all hosts are processed
19:30:20 <jtanner> i wonder if that's argparse defaulting to NoneType
19:30:23 <bcoca> its equivalent of 'dont limit'
19:30:36 <bcoca> jtanner: would be skipped from list processing
19:30:45 <bcoca> default should be empty list
19:30:48 <felixfontein> I would assume that `--limit ''` would be one less host than `--limit some.host`, i.e. limit to no host at all
19:31:01 <bcoca> --limit !all
19:31:35 <jtanner> --limit='' would imply an empty host pattern to me. My interpretation is that it should be a suitable string to put into "hosts:"
19:33:26 <felixfontein> at least with the PR, `--limit ,` behaves the same as `--limit ''`
19:33:42 <bcoca> limit is an ADDITIONAL sets of patterns to limit the host selection
19:33:44 <sdoran> Oof. I forgot about the meeting. Sorry. 😊
19:33:46 <bcoca> empty pattern = no limit
19:34:00 * sdoran catches up
19:34:05 <bcoca> felixfontein: it should and probably that was behaviour before mcperfy
19:36:09 <sivel> sdoran wrote a few new test cases, test them on 2.8 and see if they match ¯\_(ツ)_/¯
19:36:28 <felixfontein> stable-2.6 also does that behavior, so I guess it should be fine
19:36:40 <bcoca> 2.6, 2.7 ',' == ''
19:37:25 <bcoca> 2.9 is only one that fails
19:37:34 <bcoca> 2.8 is also same
19:38:38 <felixfontein> so let's get the PR merged and backported :)
19:38:52 <bcoca> added comment to it
19:39:07 <sdoran> I'll look at it and address the comment.
19:39:22 <jtanner> sdoran: do you have any notes about the behavior across releases?
19:39:32 <sdoran> No.
19:39:42 <sdoran> I just fixed the bug, didn't look back through time.
19:39:46 <sdoran> I will, though.
19:39:58 <jtanner> yeah, that's my concern that someone might have thought the bug was a feature
19:40:01 <bcoca> sdoran: another fix might be removing p instead of 'continue' in the other loop
19:40:02 <sdoran> Especially now that we have tests. 😁
19:40:24 <bcoca> jtanner: just tested, behaviour is same as far as 2.4
19:40:29 <felixfontein> 2.6, 2.7, 2.8 seem to be have as with the PR, so it should be fine
19:40:36 <bcoca> jtanner: also change was not 'feature' was performance issue .. which changed behaviour
19:40:37 <jtanner> bcoca: if you can note that in the PR that would be great
19:40:50 <sdoran> bcoca: I figured there would be some room for improvement. I appreciate feedback.
19:40:55 <jtanner> CYA n all that
19:43:32 <jillr> #topic open floor
19:43:57 <jtanner> FYI, ansiblefest is next week ... so most of these meetings will likely not happen
19:44:10 <felixfontein> makes sense
19:44:11 <jtanner> we do have contrib summit and dialins I believe
19:44:16 <jtanner> for monday
19:45:17 <sdoran> Yes, please attend the Contributors Summit on Monday.
19:45:23 <felixfontein> I plan to!
19:45:26 <sdoran> It'll be in the this channel, I believe.
19:45:35 <geerlingguy> summary post on Contributors Summit: https://github.com/ansible/community/issues/390#issuecomment-530300832
19:46:25 <jillr> afternoon working groups will use other channels, WG etherpads are linked in the main etherpad
19:47:31 <jillr> oh, Contrib Summit will be in #ansible-community
19:47:37 <jillr> https://etherpad.openstack.org/p/ansible-summit-atlanta-2019
19:49:16 <jillr> if you're attending in-person dont forget to add sessions to your agenda (it helps with space planning!)
19:52:00 <jillr> anything else for today?
19:53:58 <jillr> ok, thanks folks!
19:54:00 <jillr> #endmeeting