19:00:05 <mattclay> #startmeeting Ansible Core Public IRC Meeting
19:00:05 <zodbot> Meeting started Tue Apr 20 19:00:05 2021 UTC.
19:00:05 <zodbot> This meeting is logged and archived in a public location.
19:00:05 <zodbot> The chair is mattclay. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:00:05 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
19:00:05 <zodbot> The meeting name has been set to 'ansible_core_public_irc_meeting'
19:00:12 <mattclay> #chair Shrews
19:00:12 <zodbot> Current chairs: Shrews mattclay
19:00:19 * shertel waves
19:00:40 * Shrews waves back
19:00:46 <mattclay> #topic interpreter discovery fallback on old "listed" distros (https://github.com/ansible/ansible/pull/74152)
19:01:07 <mattclay> This is nitzmahone's topic from a previous meeting.
19:01:34 <nitzmahone> It's kind of a corner case in interpreter discovery
19:02:18 <nitzmahone> If we add an interpreter mapping entry for a distro version, and the current version is less than all of them, it uses the lowest one
19:02:56 <nitzmahone> Debian only had an entry for python3 on 10, so Debian 8 with Python3 installed ends up using that, but it's python 3.4
19:03:17 <nitzmahone> So the simple fix for that is to just add an entry for debian 8 that says /usr/bin/python
19:03:54 <nitzmahone> But the "right" fix is probably to bail out of extended discovery and use the fallback list like anything else if the current distro is older than anything in the mapping for that distro
19:04:09 <bcoca> or add 'range notation'
19:04:18 <cyberpear> adding the new entry makes sense to me... are there likely cases where there'd be an unsupported python3 installed but not python2?
19:04:30 <bcoca> both can be solutions
19:04:46 <nitzmahone> It already works as a range, but when there's only one entry...
19:05:00 <bcoca> explicit range, vs implicit
19:05:08 <mattclay> I agree that the fallback should be used if the current distro is older than the mapping for that distro.
19:05:33 <bcoca> mattclay: in this case it was in middle of 2 others
19:05:36 * nitzmahone really doesn't want to change the mapping to be explicit ranges, since we don't have a use case for that that the current thing doesn't meet
19:05:41 <nitzmahone> It wasn't
19:05:49 <mattclay> The only entry is for Debian 10 right now.
19:05:55 <bcoca> thought we had 6 also
19:06:01 <nitzmahone> nope
19:06:24 <bcoca> ah, was looking at centoss above, nvmd
19:07:07 <nitzmahone> It's only an issue if we greatly expand the use of the distro map, which I don't have any plans to do- the easy solution is just to make sure that there's an entry for the lowest version of a given distro we might reasonably encounter
19:07:52 <bcoca> debian 3?
19:08:03 <nitzmahone> hence "reasonably" ;)
19:08:25 <bcoca> you can reasonably encounter many unreasonable things
19:08:28 <bcoca> lets say 'supported'
19:09:37 <bcoca> +1 for entry now, +1 to possibly consider smarter ranges later
19:10:22 <Shrews> if we change it to use the fallback list instead of lowest available version, could that possibly change the interpreter for any current users?
19:10:32 <bcoca> also i would consider this 'bugfix' for backport policy
19:10:54 <nitzmahone> Shrews: not really- if anything it would make it work in cases where it didn't before
19:11:02 <nitzmahone> (eg the submitter's case)
19:11:09 <Shrews> *nod*
19:11:14 <bcoca> well, yes, but in the case of reporter, it woudl change to 'right' interpreter
19:11:54 <nitzmahone> Yeah, I can't think of any negative behavior that going back to the fallback would cause, other than just the usual warnings and stuff
19:12:56 <nitzmahone> I'm also leaning toward "just add the entry" for now and if we end up adding more distros for some reason, re-evaluate the fallback behavior at that point
19:13:05 <mattclay> That works for me.
19:13:36 <nitzmahone> Cool, thanks for the discussion- just wanted to make sure nobody else could think of a "hole" in that scenario I couldn't
19:14:08 <nitzmahone> (and yes, bcoca, I concur this would be a backportable bugfix)
19:14:09 <bcoca> i suspect some obscure distros are having issues
19:14:21 <bcoca> but until someone tells us about it ...
19:14:34 <mattclay> OK, anything else on this topic then?
19:14:39 <nitzmahone> Not because of this, since we don't list obscure distros- I've been pretty rigid so far about accepting new distros into that map
19:14:42 * bcoca goes back to recompile gentoo kernel
19:15:07 <nitzmahone> Nope, thanks!
19:15:13 <tterranigma> Could we discuss https://github.com/ansible/ansible/pull/74301 and https://github.com/ansible/ansible/pull/74284
19:15:33 <tterranigma> (I am @nkakouros on github)
19:15:35 <mattclay> #topic support collection playbooks in subdirs (https://github.com/ansible/ansible/pull/74301 and https://github.com/ansible/ansible/pull/74284)
19:15:49 <mattclay> bcoca: One of those PRs is yours.
19:15:56 <tterranigma> Yes
19:16:00 <tterranigma> The bad one
19:16:05 <tterranigma> :-D
19:16:08 <bcoca> im always the bad one
19:16:10 <nitzmahone> (ooh, them's fightin' words)
19:16:35 <bcoca> my pr also has 'opposite fix' in subject
19:16:50 <bcoca> just need to decide direction (will also have to update tests)
19:16:57 <tterranigma> I think supporting subdirs is useful for structure. I have a projects with about 50 playbooks. Having them all in a single directory makes it hard to maintain and for users to discover.
19:17:59 <tterranigma> It also breaks consistency since roles in collections can be in subdirs, and the mental model of namespaces coming from programming languages expects subdirs to be supported
19:18:40 <tterranigma> there was a mention of subdirs in playbooks causing trouble in some cases, could you give an example?
19:18:52 <sivel> 74301 adds subdir support fwiw, although the diff in the initial comment, has the alternative of disallowing it
19:19:16 <bcoca> im more partial to remove subdirs from roles ... but just cause i like to avoid MEGA collections (same with roles, i seee HUUUUUGE ones out there)
19:19:24 <sivel> so I mean, it sounds like we're definitely open to allowing it to work
19:20:10 <bcoca> but there is a diff between what i would encourage and what we should allow users to do ... fine here is the rope for hanging self ...
19:22:22 <sivel> but I do see it was suggested coming to the IRC meeting to talk about it. Although unsure what really needs to be discussed
19:22:43 <bcoca> mostly a vote to see if everyone agrees with enabling this
19:23:02 <bcoca> also, give chance for pro/cons to be aired and debated
19:25:05 <nitzmahone> IIRC the thing I was worried about with this initially was defining the behavior around the playbook dir, DWIM, etc- adding subdir launch support can make that even hairier,
19:25:45 <bcoca> not really dwim is always relative to playbook path, that does not change
19:26:06 <nitzmahone> But it's been long enough that I don't even remember offhand what the playbook dir is set to when launched this way
19:26:18 <bcoca> the dir the playbook is in
19:27:19 <nitzmahone> So as long as nobody's expecting any new magic with that, trying to flatten things or adding parent dirs to search paths, I guess I don't have a problem with it
19:27:50 <bcoca> only magic i expect people to ask for is TLD collection dirs for files/vars/tempaltes ... well, they have asked already, even before plays were a thing
19:31:00 <bcoca> so vote?
19:32:15 <nitzmahone> +1 for allow subdir execution of collection playbooks without further magic
19:32:29 <sdoran> +1 _ditto_
19:32:32 <nitzmahone> (basically as-implemented in 74284, plus tests)
19:32:33 <mattclay> +1 same
19:32:33 <shertel> organization is nice, +1
19:32:38 <Shrews> +1
19:32:46 <bcoca> k, can we consider bugfix and backport to 2.11?
19:34:17 <mattclay> I do need to point out one thing though. We now have 2 copies of the collection loader, so if we're making changes or bug fixes there we probably need to update both copies.
19:34:18 <nitzmahone> Maaaaybe, but it'd be in 2.11.1
19:34:33 <bcoca> works4me
19:34:49 <nitzmahone> This would be one of those changes (I think) that would never hit the old copy
19:34:50 <bcoca> mattclay: 2 copoies?!?!?
19:34:57 <bcoca> where/what/when?
19:35:19 <mattclay> bcoca: Well, two implementations. One for Python 3.8+ and one for 3.7 and earlier (only used for unit and sanity tests).
19:35:21 <nitzmahone> This is to allow us to 3.x-ify the one in product code while ansible-test can use a 2.x friendly one
19:35:28 <bcoca> ah
19:35:39 <mattclay> They are identical currently, but will soon deviate.
19:35:39 <nitzmahone> (just happened with split controller/remote stuff)
19:35:55 <bcoca> well, arent we moving to 3.8 in 2.12,s houldnt that copy go away?
19:36:09 <nitzmahone> Not if we want unit and sanity tests to work pre 3.8
19:36:09 <mattclay> They don't need to deviate in function though (and probably shouldn't in most cases), but will in implementation.
19:36:23 <mattclay> bcoca: The move is why we need 2 copies.
19:36:27 <nitzmahone> (eg, the 2.6->3.7 "compile" sanity test)
19:36:33 <nitzmahone> (and import, and others)
19:36:44 <bcoca> sigh ... i'll find copy and add changes there too
19:36:51 <mattclay> The import sanity test and all module/module_utils unit tests need the collection loader.
19:37:09 <mattclay> bcoca: https://github.com/ansible/ansible/tree/devel/test/lib/ansible_test/_data/legacy_collection_loader
19:37:22 <Shrews> oh. eww
19:37:39 <mattclay> That's what we get for having to support such a wide range of python versions.
19:37:48 <nitzmahone> This particular change shouldn't *need* to be copied there, but ...
19:37:53 <mattclay> And having different requirements between the controller and managed nodes.
19:37:58 <bcoca> or rely closely on python internals for the search/load ...
19:37:59 <nitzmahone> (I don't think any of the tests that use it will hit that code)
19:38:15 <mattclay> They shouldn't, but I don't think we should make them diverge if we don't need to (and they're identical currently).
19:38:40 <mattclay> The farther they drift, the harder it will be to keep them functionally identical if we need to change behavior or fix bugs.
19:39:25 <bcoca> agreed ... but also why i hate dupe code
19:39:42 <mattclay> I don't like it either, but we don't have a choice really.
19:40:07 <nitzmahone> Not really much else we could do, short of implementing a 3.x shim over the 2.x loader, which is what's going away in Python 3.11+
19:40:32 <nitzmahone> (or just not testing anything on Pythons < 3.8)
19:41:31 <sivel> whoa, hey, still stuff going on over here
19:41:31 <mattclay> Plus the shim would require support for older python syntax (python 2.6+ in ansible 2.12 and 2.7+ in 2.13+), despite being "controller only".
19:41:48 <sivel> got lost infinite scrolling
19:42:19 <mattclay> Which is why we went with two implementations instead.
19:42:30 <mattclay> OK, so is there anything else we need to discuss on this topic?
19:42:39 <nitzmahone> I'm good
19:43:44 <mattclay> #topic Open Floor
19:45:21 <tterranigma> Thank you for discussing this and to @bcoca for implementing, I will close my PR as the (truly) much worse one.
19:46:07 <mattclay> If there isn't anything else to discuss, I'll end the meeting in 2 minutes.
19:48:23 <mattclay> #endmeeting