19:02:07 <bcoca> #startmeeting ansible core public irc meeting
19:02:07 <zodbot> Meeting started Tue Oct 30 19:02:07 2018 UTC.
19:02:07 <zodbot> This meeting is logged and archived in a public location.
19:02:07 <zodbot> The chair is bcoca. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:02:07 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
19:02:07 <zodbot> The meeting name has been set to 'ansible_core_public_irc_meeting'
19:03:07 <bcoca> #topic https://github.com/ansible/ansible/pull/38269
19:03:17 <bcoca> @alikins_ did you finish review on that?
19:03:44 <alikins_> I'm not reviewing that
19:04:07 <bcoca> ah, for some reason i thought you had volunteered on it
19:06:14 <bcoca> hmm, just saw your recent comment, if authors/maintainers still have open questions then nothing to do here
19:06:18 <bcoca> #topic open floor
19:07:22 <maxamillion> .hello2
19:07:23 <zodbot> maxamillion: maxamillion 'Adam Miller' <maxamillion@gmail.com>
19:07:25 <felixfontein> just seeing you meet here, I got one question (or actually two) :)
19:07:47 * sdoran waves
19:08:05 <felixfontein> there's a PR for the docker inventory script (https://github.com/ansible/ansible/pull/23096), I think it looks good, but I have absolutely no knowledge about inventory scripts. can someone maybe take a quick look?
19:08:47 <bcoca> iirc we stopped taking changes for the inventory scripts since they are mostly unmaintained
19:09:31 <sdoran> Yeah, which is addressed in the PR by the author.
19:09:59 <felixfontein> and secondly, the author asks (https://github.com/ansible/ansible/pull/23096#issuecomment-434321683) about comments about a related commit (https://github.com/milo-minderbinder/ansible/commit/c5c7bac03af6370267537572979bf14a6c1154e4), which apparently was discussed on the devel ML, and where bcoca already commented on that this should probably become an inventory plugin. I know nothing about
19:10:05 <felixfontein> that (and I'm not sure if anyone of the docker_* maintainers does, at least nobody replied so far), so maybe someone else can comment on that?
19:10:21 <bcoca> change itself lgtm after quick scan
19:11:05 <felixfontein> ok, then I think it can be merged ;)
19:11:15 <bcoca> though some things are not py2/py3 safe like `str(exc) `
19:11:30 <bcoca> but they already existed ..
19:11:39 <felixfontein> yep
19:11:43 <felixfontein> it doesn't make it worse...
19:12:07 <bcoca> anyone against mergin this?
19:12:38 <sdoran> It's a sticky spot: simple fix to make the thing work today, but goes against the policy of nor more changes to scripts.
19:12:48 <sdoran> I'm ok with merging it, just be pragmatic.
19:12:52 <sdoran> *to
19:13:10 <felixfontein> merging it is also encouraging the first-time contributor to maybe do something more :)
19:13:47 <felixfontein> is there already a docker-related inventory plugin?
19:13:50 <sdoran> The good outweighs the bad. And asking "could you just please rewrite this whole thing?" is probably not helpful.
19:14:24 <felixfontein> +1
19:18:20 <bcoca> set rebuild_merge, since no oppo
19:19:06 <felixfontein> thanks!
19:19:30 <felixfontein> about the author's question: "Anyway, let me know if the dynamic inventory group/host naming issue still seems relevant and if I should create a separate issue & PR, or if this problem has been addressed through one of the ansible core plugins or modules."
19:19:41 <felixfontein> (https://github.com/ansible/ansible/pull/23096#issuecomment-434321683)
19:23:05 <bcoca> idk
19:23:11 * gundalow waves
19:24:28 <cyberpear_> can we talk about raw_stdin?
19:24:52 <cyberpear_> https://github.com/ansible/ansible/pull/45170
19:25:08 <bcoca> felixfontein:  im sure we'll eventually end with a  docker inventory plugin, until then i would say .. we'll take each PR as it pops up
19:25:17 <bcoca> cyberpear_: wait till prev topic is done
19:25:28 * cyberpear_ will wait, sorry
19:26:40 <bcoca> felixfontein: anything else on this?
19:26:55 <felixfontein> bcoca: I don't think so. I'll paraphrase your answer in the PR, if you don't mind :)
19:27:52 <bcoca> k,
19:27:57 <bcoca> #topic https://github.com/ansible/ansible/pull/45170
19:28:52 <felixfontein> thanks everyone!
19:28:55 <bcoca> -1 what you really seem to want is to modify current behaviour of stdin, not add an alternate
19:29:09 <bcoca> also 'binary_data' will be a misnomer as its not possible to pass from a playbook this way
19:29:43 <cyberpear_> would you be okay with modifying the behavior, even with it markedas 'stableinterface'?
19:30:06 <bcoca> no, add an option that does it, defaults to current, not a parallel option
19:30:17 <cyberpear_> that's what I've done
19:30:19 <bcoca> i.e nonposix: true <= avoids the line ending issue
19:30:36 <cyberpear_> raw_stdin is a binary flag that tweaks the behavior
19:30:42 <bcoca> nvmd, i misread what raw_stdin is ... name is misleading
19:30:44 <cyberpear_> but I'm fine renaming it to 'nonposix'
19:31:11 <cyberpear_> I'm fine w/ any name that's preferred.
19:31:22 * bcoca this is why i wanted to avoid stdin in the first place
19:31:50 * cyberpear_ finds stdin as useful for passwords, etc.
19:33:15 <bcoca> jborean93: you already reviewed, any reason its not merged?
19:33:27 <bcoca> i still think name is misleading, but dont really have good alternative
19:33:57 <jborean93> It used to be binary_stdin to match run_command IIRC and this was the best one we could come up with
19:34:21 <jborean93> I mostly forgot about it but I believe I didn’t merge at the time because I was waiting on someone else to review
19:35:02 <bcoca> code lgtm, its just the name  i found confusing
19:35:14 <bcoca> stdin_posix: yes|no ?
19:35:38 <jborean93> The trouble is this could potentially be useful for win_* modules where posix isn’t a thing
19:35:55 <bcoca> the whole reason a \n gets added is for 'valid posix file'
19:36:00 <jborean93> I can’t remember if we already append a newline or not though
19:36:12 <bcoca> i expect a \r\n on windows
19:36:48 <nitzmahone> IIRC we just close the stdin handle
19:36:50 <cyberpear_> I'd hope that it would just take what was literally passed as the arg, without modification.
19:37:00 <bcoca> if not binary_data:
19:37:02 <bcoca> data += '\n'
19:37:05 * nitzmahone looks
19:37:57 <jborean93> I don’t think we explicitly add any new lines
19:38:07 <jborean93> Unless we call writeline on the stdin pipe created
19:38:29 <bcoca> on windows side, ^ code above is 'non windows'
19:38:57 <jborean93> I know, I’m just saying stdin_posix doesn’t really match for Windows if we need to implement the arg there
19:39:34 <bcoca> add_line_ending?
19:39:53 <nitzmahone> Yeah, that's more what I was thinking `add_newline_to_stdin`
19:40:07 * bcoca opens up bikeshed for voting
19:40:12 * nitzmahone hates generically-named args like "force"
19:42:26 * cyberpear_ likes raw_stdin as simplest
19:46:02 <bcoca> cyberpear_: my issue with that is people will put the 'raw stdin content' in it
19:46:09 <bcoca> instead of thinking of it as a toggle
19:46:14 <nitzmahone> Yeah, but `raw` meaning not escaped? not encoded?
19:46:23 <bcoca> ^ also that
19:46:42 <bcoca> you cannot pass binary as templating/jinja2 and yaml will convert to text
19:46:52 <bcoca> so 'raw' is double misleading
19:47:22 <cyberpear_> you can put binary into yaml, though it's not common
19:48:31 <cyberpear_> literal_stdin?
19:48:45 <cyberpear_> stdin_no_munge?
19:49:14 <jborean93> `stdin_newline`?
19:49:31 <jborean93> `add_newline_to_stdin` seems too long but really I don't mind either way
19:50:08 <bcoca> cyberpear_: yes,b ut that requires knowing !!binary and the parser/language supporting it
19:50:12 <nitzmahone> stdin_newline doesn't say "boolean" to me
19:50:32 <bcoca> even then it would not survive passing to module as that gets serialized/deserialized
19:50:34 <cyberpear_> so if we pick something that has 'no' in it, it would hint at binary
19:50:41 <cyberpear_> (boolean, rather)\
19:50:42 <bcoca> would need to add base64 encode/decode
19:51:02 <bcoca> no_ending_added
19:51:07 <nitzmahone> no_stdin_trailing_newline
19:51:26 * nitzmahone also usually dislikes double-negative args
19:51:36 <bcoca> #note code to fix issue not hard, naming .... impossible!
19:51:37 <nitzmahone> `no_stdin_trailing_newline: no`
19:51:41 <nitzmahone> (head explodes)
19:52:00 <cyberpear_> (we've already got that with 'no_log: no' :P)
19:52:29 <bcoca> tempted to change that to 'censor'
19:52:49 <jborean93> what about `no_not_censor` :P
19:52:53 <cyberpear_> lol
19:53:10 <bcoca> k, back to current bikeshed, before we rewrite all keywords
19:53:22 <jborean93> out of all I think `add_newline_to_stdin` is the best if we aren't worrying about the length
19:53:25 * bcoca stares at ignore_errors
19:53:54 <bcoca> skip_appending_newline
19:55:08 <cyberpear_> https://github.com/ansible/ansible/blob/ddfd1dbfc614a7a46192aa77618af36a58dda020/lib/ansible/module_utils/basic.py#L2711 is the description of the newline-adding, and https://github.com/ansible/ansible/blob/ddfd1dbfc614a7a46192aa77618af36a58dda020/lib/ansible/module_utils/basic.py#L2873 is the implementation
19:55:19 <cyberpear_> stdin_add_newline?
19:55:51 <cyberpear_> ^seems most descriptive, and is not negative
19:55:53 <bcoca> tempted to just do poll on ML .. but mcnewlineface will probably win
19:56:06 <nitzmahone> stdin_add_newline WFM
19:56:14 <bcoca> we kind of want negative for the defeault no/None
19:56:28 <nitzmahone> meh
19:56:30 * cyberpear_ fine w/ default=yes
19:56:33 <bcoca> having a 'true' default  will not let you know if user set or not
19:56:40 <jborean93> I'm fine with `stdin_add_newline`
19:56:46 <bcoca> but im going to say good enough in this case
19:56:51 <jborean93> isn't this defautled to yes anyway?
19:56:54 <bcoca> stdin_add_newline wins by exhaustion
19:57:03 <bcoca> jborean93: the point was to default to no/none
19:57:11 <bcoca> which would still append line
19:57:14 <jborean93> ah
19:57:14 <cyberpear_> the current behavior would be `stdin_add_newline: yes`
19:57:23 <bcoca> ok with that
19:57:27 <nitzmahone> +1
19:57:30 <bcoca> +1
19:57:51 <bcoca> #topic open floor
19:58:53 * cyberpear_ will update PR for stdin_add_newline
19:59:25 <bcoca> #endmeeting