19:00:32 <sdoran> #startmeeting Ansible Core IRC meeting
19:00:32 <zodbot> Meeting started Tue Feb 12 19:00:32 2019 UTC.
19:00:32 <zodbot> This meeting is logged and archived in a public location.
19:00:32 <zodbot> The chair is sdoran. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:00:32 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
19:00:32 <zodbot> The meeting name has been set to 'ansible_core_irc_meeting'
19:00:38 <nitzmahone> o/
19:00:49 <sdoran> #info Agenda https://github.com/ansible/community/labels/core
19:00:52 <sdoran> #chair nitzmahone
19:00:52 <zodbot> Current chairs: nitzmahone sdoran
19:00:53 * shertel waves
19:01:04 <sdoran> #chair shertel
19:01:04 <zodbot> Current chairs: nitzmahone sdoran shertel
19:02:05 <sdoran> #topic followup discussion about possibly doing SSH-like-only connection method first, in order to flush out the `ansible_python_interpreter` task_vars issue
19:02:14 <maxamillion> .hello2
19:02:15 <zodbot> maxamillion: maxamillion 'Adam Miller' <maxamillion@gmail.com>
19:02:22 <sdoran> #chair maxamillion
19:02:22 <zodbot> Current chairs: maxamillion nitzmahone sdoran shertel
19:02:27 <gundalow> hum, don't see dw here yet
19:02:38 <sdoran> #chair gundalow
19:02:38 <zodbot> Current chairs: gundalow maxamillion nitzmahone sdoran shertel
19:02:39 <nitzmahone> yeah, might want to open floor or anything else first
19:03:02 <sdoran> Ok, moving to next topic until dw is around.
19:03:10 <sdoran> #topic https://github.com/ansible/ansible/pull/51768
19:03:20 <gundalow> +1
19:03:21 <sdoran> This is from dag
19:03:22 * agaffney is here
19:03:33 <sdoran> #chair agaffney
19:03:33 <zodbot> Current chairs: agaffney gundalow maxamillion nitzmahone sdoran shertel
19:03:37 <agaffney> when dag and I agree, you know it's something good ;)
19:03:47 <sdoran> oooo boy
19:04:02 <maxamillion> SHIPIT
19:04:18 <nitzmahone> Yeah, I can't come up with a situation where this could break anything- if nobody else can either, it's an excellent change
19:05:01 <gundalow> agaffney: thanks for adding the Porting Guide
19:05:09 * sdoran reading through it
19:06:44 <sdoran> This is very nice. Even has tests.
19:07:01 <agaffney> I wanted to make damn sure that it wouldn't break anything
19:07:39 <sdoran> Someone will always find a way to break it. :)
19:07:54 <nitzmahone> Yeah- it's a big change to the signal path, but until someone comes up with a concrete fail...
19:07:54 <shertel> Nice, simple way of fixing it  :-) +1 from me, though I feel like the last time this was discussed people weren't agreeing on the desired behavior (i.e. expecting some things should fail) and this would break their expectations.
19:08:38 <nitzmahone> shertel: got any specifics? I couldn't come up with anything realistic
19:09:27 <shertel> dag mentioned ignore_errors
19:09:32 <nitzmahone> *worst* case, maybe we add a config switch to disable the behavior, but until someone comes up with something remotely plausible...
19:09:40 <shertel> +1
19:09:52 <nitzmahone> I don't think this would even interact with `ignore_errors` off the top of my head
19:10:10 <sdoran> +1
19:10:17 <nitzmahone> (would be a task templating failure)
19:11:17 <sdoran> I'll merge this after tests pass.
19:11:21 <felixfontein> cool!
19:11:23 <shertel> Hm... okay. I haven't played with it. Does it affect expecting things to be undefined?
19:11:53 <agaffney> I just rekicked the previously failed tests. they don't appear to be related
19:11:55 <nitzmahone> shertel: if you're not familiar with null-conditional operators, this basically turns "." into one
19:11:58 <sdoran> #action Merge PR #51768 once tests pass
19:12:05 <GundalowBarker[m> thanks sdoran
19:12:39 <nitzmahone> so the end result will still be Undefined, but it lets you dot through multiple layers of undefined to do things like `default` without intermediate layers
19:12:46 <agaffney> shertel: it shouldn't. if 'foo' is undefined, 'foo.bar.baz' will also be undefined
19:12:47 <nitzmahone> I'm not explaining it very well
19:13:29 <nitzmahone> it just prevents the first part from bombing with a null reference instead of returning undefined
19:13:32 <agaffney> all this really does it remove the extra hoops you have to jump through to handle multiple levels of undefined values
19:14:47 <sdoran> Ok, moving on to our next topic.
19:14:57 <sdoran> #topic https://github.com/ansible/ansible/pull/46687
19:15:59 <nitzmahone> this makes me nervous with collections changes
19:17:16 <nitzmahone> if it's purely informational and would always have the fully-qualified role name, it'd probably be fine, but we might want to hold off merging it until collections/role stuff is merged to devel, then revisit any potential weirdness
19:17:27 <sivel> I feel like `v.setdefault('ansible_parent_role_names', []).insert(0, self._parent_role.get_name())` is backwards from what I would expect
19:17:36 <nitzmahone> (otherwise we're potentially adding complexity/delays/brokenness)
19:17:42 <sdoran> #chair sivel
19:17:42 <zodbot> Current chairs: agaffney gundalow maxamillion nitzmahone sdoran shertel sivel
19:17:59 <maxamillion> sivel: how so?
19:18:24 <sivel> I might just be not looking closely enough
19:18:39 <maxamillion> nitzmahone: I like the idea, but I do worry about what it could mean for content ... I'd be interested in input from the galaxy/mazer folks
19:18:42 <sivel> My initial thought was that the most direct parent would be at the end, instead of the front
19:18:45 <nitzmahone> sivel: you mean that the most recent parent is at the top?
19:19:03 <sdoran> sivel: Agreed. I felt that was upside down.
19:19:20 <nitzmahone> maxamillion: yeah, I'm not opposed to the idea either (we don't do a great job of surfacing meta-exec data like this today)
19:19:31 <maxamillion> oh, that's how I expected it to be ordered ... not like a stack, but like a list ... most direct parent first
19:19:37 <sivel> but looking closer, I may just be wrong, as that code looks like it inserts in reverse order
19:20:00 <sivel> *shrug* I'd have to see the output
19:20:15 <nitzmahone> at a glance it looks like what I'd expect (most current parent at the head of the list)
19:20:24 <maxamillion> +1
19:20:26 <sdoran> The docs state the most recent role is the first item in the list. The code matches that.
19:20:33 <sdoran> Still seems upside down to my mind.
19:20:55 * nitzmahone shudders to think what kinds of things people would use this for
19:21:14 <sivel> and if we have `names` and the parent is in [0], then why do we really need `name`?
19:21:56 <sdoran> Nitpicking aside, what are the implications of adding this?
19:22:16 <sdoran> Seems like a very odd thing to need.
19:22:27 <nitzmahone> Potentially useful forensic data, potential gun-pointed-at-foot for more dynamic role behavior... ;)
19:22:33 <sdoran> Though I generally frown upon including roles from roles to avoid creating block holes. :)
19:22:42 <sdoran> *black
19:23:26 <sdoran> sivel: I imagine `name` is just a convenience variable so folks don't have to use array addresses or the `| first` filter.
19:23:28 <maxamillion> it happens more often in the wild than I ever thought it would even if it's not super common
19:23:31 <sdoran> So more UI than anything.
19:23:53 <cyberpear> I've got several roles that act like tasks, and do in fact include_role (partly because I haven't yet learned to make an ansible modlue from scratch)
19:25:47 <cyberpear> (and I've got a family of openstack roles that call each other, such as the project-creation role calling the network-creation role)
19:27:05 <cyberpear> does this PR work for roles that use `meta: dependencies` to include other roles?
19:30:22 <sdoran> I am not sure.
19:30:35 * cyberpear should go test...
19:31:05 <sdoran> Any change to role import/include involves a myriad of use cases. It is very hard to figure out all the different scenarios you can run into.
19:31:08 <cyberpear> would be good to have a list of parent_role_paths, to be consistent w/ names
19:31:38 <cyberpear> (comment to that effect added to PR)
19:33:11 <sdoran> nitzmahone: Should we hold off on this until we can determine how this works with collections?
19:33:59 <nitzmahone> Looking at the impl, I don't *think* it should complicate things further, just a little extra test coverage to be sure
19:34:31 <nitzmahone> but there are some UI things to think about (eg, just have lists, no scalar versions)
19:34:42 <cyberpear> Is collections this: https://github.com/ansible-collections ?
19:34:55 <sdoran> We should determine if this is something we want before asking the author to rebase again.
19:35:29 <sdoran> cyberpear: No, not at all.
19:35:32 * nitzmahone is +0.5 - as with many things, it can be useful, but could also probably be abused
19:36:06 * cyberpear could benefit from this feature
19:36:32 <maxamillion> nitzmahone: I think I'm on board with list-only and no scalars
19:36:59 <cyberpear> I currently use the inheritance feature that roles included with meta:dependencies will look at their "parent" roles' files/ and templates/ dirs
19:37:24 <maxamillion> cyberpear: I'm actually not sure what that org is or those repos are, but a collection is as mazer/galaxy define it here: https://github.com/ansible/mazer
19:37:44 <cyberpear> (haven't checked if that works w/include_role)
19:37:54 <cyberpear> maxamillion: thanks for the reference
19:38:14 <maxamillion> cyberpear: short version, it's a set of any one or many of Ansible roles, modules, module_utils, or plugins
19:38:23 <maxamillion> cyberpear: with some metadata sprinkled on top
19:38:24 <sdoran> cyberpear: Here is an example from the mazer tests: https://github.com/ansible/mazer/blob/8bde6df0d79c6f4a5d7e70542c0e8f4ce8e2dd3e/tests/ansible_galaxy/example_artifact_manifest1.yml
19:39:38 <cyberpear> presumable you'd be able to use those w/o having to include them in a 'roles' section?
19:40:45 <nitzmahone> they work like any other roles, more about the ability to version/distribute in groups, and trying to make plugin content a first-class citizen in galaxy (roles suck for distributing plugins for a number of reasons)
19:41:16 <cyberpear> (right now, I have some utility roles that I hack into place by using symlinks so they can be used from command line, not just in playbooks)
19:41:28 <cyberpear> thanks for the explanations, sorry for the derailment...
19:41:32 <sdoran> It seems like we don't have a strong consensus adding this change.
19:41:41 <nitzmahone> So you'd have the concept of "installed roles" with your controller, but you can also install collection content playbook-adjacent similar to how it works today
19:42:22 <nitzmahone> sdoran: yeah, if it goes list-only, and we can wait to merge it til after collections are in devel, I'm ok with it
19:43:00 <cyberpear> (sounds sort of like 'yum install rhel-system-roles' then using those, but generic)
19:43:23 <sdoran> #info Revisit PR #46687 once collections are merged into `devel`, and possibly change it to be list-only.
19:43:49 <sdoran> #topic https://github.com/ansible/ansible/pull/51499
19:44:23 <nitzmahone> I'm in the "command-warnings are lame and we should just kill them" camp, but ...
19:46:49 <sdoran> I made this PR in response to an issue.
19:47:15 <sdoran> Someone was using `shell` to reboot a system and having an issue.
19:47:26 <sdoran> So I figured it may be helpful to suggest using a module.
19:47:48 <sdoran> Though I ended up having to do gymnastics in the action plugin.
19:47:51 <nitzmahone> The cure is worse than the disease IMO
19:48:01 <cyberpear> I'm generally okay w/ command warnings, but I most often run into it when checking package installation using 'rpm -q mypackage' rather than the package/yum module because the former is much faster.
19:48:07 <sdoran> It ended up being pretty complex to make it work.
19:48:34 <cyberpear> for 'reboot' in particular, I'm in favor of the change because the old way of doing it had 1000 forms, most of which were broken in some way.
19:49:59 <cyberpear> (my favorite being 'shell: nohup sh -c "sleep 5 ; shutdown -r now" > /dev/null &')
19:50:06 <maxamillion> I like the command warnings, but maybe I'm a minority in that
19:51:51 <maxamillion> or maybe that functionality could be relocated to ansible-lint?
19:52:04 <nitzmahone> just hate putting all that regex goo into the command action path
19:52:16 <sdoran> My only other concern with this PR is this line: https://github.com/ansible/ansible/pull/51499/files#diff-82a949ca64b4588912ef7913b1c5c8abR33
19:52:22 <sdoran> Jinx!
19:52:25 <maxamillion> I'd be cool with that, I just like *something* to be like "hey, in case you didn't know ..." because I'd wager that not *everyone* reads the release notes and might not know about new modules, such as the `reboot` module
19:52:27 <nitzmahone> Yeah, I think that kind of thing belongs in opt-in tools
19:52:40 <maxamillion> nitzmahone: +1 - I think that's fair
19:53:17 <nitzmahone> sdoran: yeah, the problem there is that because it's post-fork, you're not getting any benefit from the compiled re (even if you kept it around)
19:53:20 <sdoran> It was somewhat tricky to figure out "is a reboot/shutdown command being issued" since in is almost always chained somehow and not just the first argument.
19:54:19 <sdoran> Ok. I'll just pare this down to some docs suggestions rather than pollute the action plugin with all this complexity.
19:54:25 <nitzmahone> +1
19:54:34 * cyberpear assumes most don't read the release notes and just complain when things break...
19:54:55 <agaffney> cyberpear: that's me, and I know better
19:55:09 <nitzmahone> and in general I think the idea of ansible-lint flagging "you should probably be using module X" is a better long-term plan than command warnings
19:55:16 <sdoran> #action sdoran to change PR #51499 to be docs-only
19:55:19 <maxamillion> eh, I think we're all guilty of not reading docs at one time or another
19:55:43 <sdoran> I still think docs is a better trade-off than the complexity this introduces.
19:56:02 <sdoran> It was one of those experiences where I stepped back after I made it work and was a bit horrified at what I had wrought. :)
19:56:10 <agaffney> heh
19:56:25 <sdoran> But it was a fun exercise.
19:57:01 <sdoran> Looks like that is all for today. Hopefully we'll get to discuss dw's topic next meeting.
19:57:01 <nitzmahone> "just because you can..." ;)
19:57:02 <sivel> Nearly at ti
19:57:07 <sivel> time*
19:57:14 <sdoran> #topic open floor
19:57:23 <sdoran> For two more minutes. :)
19:57:33 <sdoran> We have a hard stop at the top of the hour today.
19:58:22 <cyberpear> Would be useful to have a way to parse arbitrary ini, json, or xml on the controlled machine into a fact-like object
19:58:58 <sdoran> Existing filters don't work for that?
19:59:10 <cyberpear> currently, I'm overriding the ansible_local functionality to cover the json and ini cases, but I'm using a hack of making a symlink ending in .fact to make it go
19:59:42 <cyberpear> haven't figured a good way to do it for xml, though I've had limited success with the 'xml' module
20:00:04 <sdoran> #endmeeting