19:00:05 #startmeeting Ansible Core Public IRC Meeting 19:00:05 Meeting started Tue Apr 20 19:00:05 2021 UTC. 19:00:05 This meeting is logged and archived in a public location. 19:00:05 The chair is mattclay. Information about MeetBot at http://wiki.debian.org/MeetBot. 19:00:05 Useful Commands: #action #agreed #halp #info #idea #link #topic. 19:00:05 The meeting name has been set to 'ansible_core_public_irc_meeting' 19:00:12 #chair Shrews 19:00:12 Current chairs: Shrews mattclay 19:00:19 * shertel waves 19:00:40 * Shrews waves back 19:00:46 #topic interpreter discovery fallback on old "listed" distros (https://github.com/ansible/ansible/pull/74152) 19:01:07 This is nitzmahone's topic from a previous meeting. 19:01:34 It's kind of a corner case in interpreter discovery 19:02:18 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 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 So the simple fix for that is to just add an entry for debian 8 that says /usr/bin/python 19:03:54 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 or add 'range notation' 19:04:18 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 both can be solutions 19:04:46 It already works as a range, but when there's only one entry... 19:05:00 explicit range, vs implicit 19:05:08 I agree that the fallback should be used if the current distro is older than the mapping for that distro. 19:05:33 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 It wasn't 19:05:49 The only entry is for Debian 10 right now. 19:05:55 thought we had 6 also 19:06:01 nope 19:06:24 ah, was looking at centoss above, nvmd 19:07:07 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 debian 3? 19:08:03 hence "reasonably" ;) 19:08:25 you can reasonably encounter many unreasonable things 19:08:28 lets say 'supported' 19:09:37 +1 for entry now, +1 to possibly consider smarter ranges later 19:10:22 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 also i would consider this 'bugfix' for backport policy 19:10:54 Shrews: not really- if anything it would make it work in cases where it didn't before 19:11:02 (eg the submitter's case) 19:11:09 *nod* 19:11:14 well, yes, but in the case of reporter, it woudl change to 'right' interpreter 19:11:54 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 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 That works for me. 19:13:36 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 (and yes, bcoca, I concur this would be a backportable bugfix) 19:14:09 i suspect some obscure distros are having issues 19:14:21 but until someone tells us about it ... 19:14:34 OK, anything else on this topic then? 19:14:39 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 Nope, thanks! 19:15:13 Could we discuss https://github.com/ansible/ansible/pull/74301 and https://github.com/ansible/ansible/pull/74284 19:15:33 (I am @nkakouros on github) 19:15:35 #topic support collection playbooks in subdirs (https://github.com/ansible/ansible/pull/74301 and https://github.com/ansible/ansible/pull/74284) 19:15:49 bcoca: One of those PRs is yours. 19:15:56 Yes 19:16:00 The bad one 19:16:05 :-D 19:16:08 im always the bad one 19:16:10 (ooh, them's fightin' words) 19:16:35 my pr also has 'opposite fix' in subject 19:16:50 just need to decide direction (will also have to update tests) 19:16:57 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 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 there was a mention of subdirs in playbooks causing trouble in some cases, could you give an example? 19:18:52 74301 adds subdir support fwiw, although the diff in the initial comment, has the alternative of disallowing it 19:19:16 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 so I mean, it sounds like we're definitely open to allowing it to work 19:20:10 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 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 mostly a vote to see if everyone agrees with enabling this 19:23:02 also, give chance for pro/cons to be aired and debated 19:25:05 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 not really dwim is always relative to playbook path, that does not change 19:26:06 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 the dir the playbook is in 19:27:19 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 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 so vote? 19:32:15 +1 for allow subdir execution of collection playbooks without further magic 19:32:29 +1 _ditto_ 19:32:32 (basically as-implemented in 74284, plus tests) 19:32:33 +1 same 19:32:33 organization is nice, +1 19:32:38 +1 19:32:46 k, can we consider bugfix and backport to 2.11? 19:34:17 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 Maaaaybe, but it'd be in 2.11.1 19:34:33 works4me 19:34:49 This would be one of those changes (I think) that would never hit the old copy 19:34:50 mattclay: 2 copoies?!?!? 19:34:57 where/what/when? 19:35:19 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 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 ah 19:35:39 They are identical currently, but will soon deviate. 19:35:39 (just happened with split controller/remote stuff) 19:35:55 well, arent we moving to 3.8 in 2.12,s houldnt that copy go away? 19:36:09 Not if we want unit and sanity tests to work pre 3.8 19:36:09 They don't need to deviate in function though (and probably shouldn't in most cases), but will in implementation. 19:36:23 bcoca: The move is why we need 2 copies. 19:36:27 (eg, the 2.6->3.7 "compile" sanity test) 19:36:33 (and import, and others) 19:36:44 sigh ... i'll find copy and add changes there too 19:36:51 The import sanity test and all module/module_utils unit tests need the collection loader. 19:37:09 bcoca: https://github.com/ansible/ansible/tree/devel/test/lib/ansible_test/_data/legacy_collection_loader 19:37:22 oh. eww 19:37:39 That's what we get for having to support such a wide range of python versions. 19:37:48 This particular change shouldn't *need* to be copied there, but ... 19:37:53 And having different requirements between the controller and managed nodes. 19:37:58 or rely closely on python internals for the search/load ... 19:37:59 (I don't think any of the tests that use it will hit that code) 19:38:15 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 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 agreed ... but also why i hate dupe code 19:39:42 I don't like it either, but we don't have a choice really. 19:40:07 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 (or just not testing anything on Pythons < 3.8) 19:41:31 whoa, hey, still stuff going on over here 19:41:31 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 got lost infinite scrolling 19:42:19 Which is why we went with two implementations instead. 19:42:30 OK, so is there anything else we need to discuss on this topic? 19:42:39 I'm good 19:43:44 #topic Open Floor 19:45:21 Thank you for discussing this and to @bcoca for implementing, I will close my PR as the (truly) much worse one. 19:46:07 If there isn't anything else to discuss, I'll end the meeting in 2 minutes. 19:48:23 #endmeeting