18:58:37 #startmeeting ansbile core irc public meeting 18:58:37 Meeting started Tue Sep 29 18:58:37 2020 UTC. 18:58:37 This meeting is logged and archived in a public location. 18:58:37 The chair is bcoca. Information about MeetBot at http://wiki.debian.org/MeetBot. 18:58:37 Useful Commands: #action #agreed #halp #info #idea #link #topic. 18:58:37 The meeting name has been set to 'ansbile_core_irc_public_meeting' 18:59:00 #topic 71928 18:59:36 o/ 18:59:46 hey! 18:59:48 \o 18:59:49 so we went over that in triage, left info on ticket, so going to skip 18:59:57 (damn, already 19:00... time flies!) 19:00:02 so we went over that in triage, left info on ticket, so going to skip 19:00:08 #topic https://github.com/ansible/community/issues/560#issuecomment-700723990 19:00:29 bcoca: did you see my comment in https://github.com/ansible/ansible/pull/71928 ? 19:00:51 felixfontein: I just added a comment too :-) 19:00:56 what about #71679? 19:01:27 shertel: reloading... 19:01:57 felixfontein: in any case, environment should not be a shell option as it is a playbook keyword 19:02:04 shertel: I used `dict or list` precisely because it skips validation, so it won't break anything :) the code which reads the option accepts both dicts and lists, so I didn't want to change it to list 19:02:39 bcoca: so how should this be fixed then? 19:02:42 we actually need to remove that from docs 19:02:48 I don't know why we'd want to skip validation, it should be a list 19:02:50 as its incorrect and never used 19:03:00 type: raw for 'plugin deals with validation' 19:03:05 bcoca: wouldn't that break backwards compat? 19:03:10 just as we do with modules 'type: dict or list' is wrong 19:03:17 shertel: with what? afaik its never used 19:03:26 it's coming from _set_connection_options, just like the `timeout` fun I recently fixed 19:03:35 3rd party plugin could be using it 19:04:32 "show ansible version" seems like a good idea to me (but the topic seems to have moved on) 19:04:36 sigh ... actually implemented inbase and just slurping the keyword ... so it can stay 19:04:44 moved back, mostly 19:04:49 cyberpear: we haven't got there yet 19:04:50 sorry :) 19:04:56 felixfontein: type:raw in any case 19:05:14 shertel: is raw ok for you? 19:05:30 why isn't `type: list` okay? 19:05:50 its not dual? 19:06:15 it's always a list, isn't it? 19:06:26 ah, no, defined as likst in play, though t it was live vars_prompt which is dual 19:06:33 k, then list, easier 19:06:33 it's a list by default, but I'm not sure whether it's always a list 19:06:52 would error out at play level otherwise 19:06:55 it should always be a list, or the FieldAttribute isn't getting validated somehow 19:07:08 the code which comsumes it accepts both list and dict 19:07:28 felixfontein: that is the plugin code, but to get there it must pass play def, and that is type=list 19:07:54 I'm fine with changing it to `type: list`, if you're sure that dict cannot happen :) 19:07:55 only current way of setting it is via list 19:07:59 try it 19:08:20 will do. 19:08:35 i get syntax error 19:08:48 I think you can set a dict, but it's turned into a list before it gets to the shell plugin because that's the type here https://github.com/ansible/ansible/blob/3e9943bc5e7a9cd393757aa8100d7fed80bd316e/lib/ansible/playbook/base.py#L608 19:09:28 if plugin had other methods of setting (say ansilbe_environment) then yes, dual nature, but as is, only 1 way and that way is always list 19:10:28 I'll change the default to `[{}]` then 19:10:40 +1 19:10:50 roger 19:10:54 k, now realy 19:11:10 #topic https://github.com/felixfontein 19:11:14 lol 19:11:26 Let's all talk about Felix! ;) 19:11:27 what do you want to discuss about me? :) 19:11:41 not sure i understand this, --version already shows version 19:12:01 it shows ansible-base version, not ansible version 19:12:25 ah, yes, cause its part of ansible-base 19:12:32 or ansible-core ... 19:12:38 users type `ansible --version` after installing ansible to see which version of ansible they have, and they are told `ansible 2.10.1`, but they just installed Ansible 2.10.0 19:12:40 whatever we name it in the end 19:12:44 I'm -1 on this one, since the version of the set of collections installed to site-packages is not that useful.. What you really need to know is the version of the collection that's actually loaded, and that's not something we can easily do right now. 19:12:50 which is confusing (but correct, since it is the version of the `ansible` CLI tool) 19:12:59 yeah, that would require the dependency to be aware of the dependant 19:13:08 yep 19:13:23 it would be nice to at least show the "real" Ansible version in the list of other versions below 19:13:24 well, just another way to reflect that keeping the ansible name was confusing 19:13:40 for some definition of 'real' 19:13:46 The reason `ansible` >=2.10 looks like it does is because the community team didn't want any "special" behavior, and that would definitely qualify 19:13:56 bcoca: yep :) 19:14:33 If anything, we should probably clarify in the base version output that this is the ansible-base version 19:14:43 (though not sure the best way to do that) 19:14:57 specially since we are about to rename it again 19:15:06 oh, are we? 19:15:47 seems people dont like 'base' 19:16:16 ok 19:17:12 I'm not sure how to improve the `ansible --version` output without completely changing it 19:17:36 no clue 19:17:40 Without tipping my hand too much, that's one of the first `ansible-chaos` projects ;) 19:17:50 can we please rename that 19:18:06 last thing a SA wants to hear is about chaos 19:18:19 SAs shouldn't be using it so ;) 19:18:54 he, arent they part of those we want to test the new things? 19:18:57 `ansible-cli`? `ansible-core`? 19:19:14 The latter seems to the most favored 19:19:39 seems a good choice 19:19:49 yep 19:20:09 IIRC the current name was foisted on us by someone that's no longer around 19:20:34 many things ... 19:21:36 * resmo eats popcorn 19:22:36 Anyway, I think we should work on clarifying what the version displayed actually is, but I'm -1 for adding `ansible` version to that output, as it's not actually useful for much IMO 19:22:46 ok. I guess there's not much we can do about the name right now, so what's about the topic, i.e. making it clearer that the version printed isn't the Ansible version, but the ansible-base/core/... version? 19:23:04 but it is the version of 'ansible' the cli script 19:23:22 Yeah- I'll add that to the chaos branch where I'm doing other stuff. I think I'll be showing that at contrib summit in a couple weeks 19:23:28 why the naming chosen is very confusing, might not be version of pypi package, but that is not what you execute when you do `ansible` at command line 19:23:38 If people like it, we can bring it over to devel 19:23:53 sounds good to me 19:24:47 #topic open floor 19:25:11 bcoca: you skipped https://github.com/ansible/ansible/pull/71679 19:25:26 no, i wwent from last entries 19:25:35 is that just waiting on another review/merge? I can add it to my queue 19:25:42 since previous logs were not posted .. hard to know what was discussed 19:25:57 bcoca: it has no checkmark :) 19:26:00 ^ i believe sivel was looking at that one 19:26:06 felixfontein: no checkmarks in w3m 19:26:07 s'ok by me 19:26:26 shertel: I thought it might need some approval that collection versions should be enfored a bit more to be semver 19:26:55 besides that, I won't mind review/merge ;) 19:27:00 felixfontein: tehy are enforced at galaxy level iirc 19:27:14 Good to catch it before that though 19:27:31 yep, because galaxy doesn't really care about version_added / removal versions 19:27:32 and if someone doesn't like it, they can always ignore those checks 19:27:48 (for their own collections anyway) 19:27:49 nitzmahone: exactly, they have names which can be ignored if really necessary 19:28:04 So yeah, +1 from me on that PR 19:28:04 not saying that as something agasint the check, just pointing out where its enforced 19:28:32 Galaxy's only checking the collection versions though- this is docs/runtime warning stuff 19:28:52 not runtime, ansible-test 19:29:00 at rurntime we dont load semver afaik 19:29:11 ansible-test's validation of docs/runtime stuff, is what I'm saying 19:29:31 'runtime' has two meanings, same as 'ansible' ;) 19:29:33 looks good to me 19:29:39 Galaxy is blissfully unaware of what this test checks for 19:31:00 does galaxy do anything except looking at the collection version number? 19:31:17 we need to stop overloading terms .. like timeout ... 19:31:17 I don't think so 19:31:40 :-) 19:32:14 * nitzmahone wonders how many collections are already doing one of the verboten behaviors 19:32:33 * bcoca bets on >50% 19:32:35 nitzmahone: probably too many 19:32:37 Guess there's only one way to find out :) 19:32:42 I don't want to know how many of these are included in ansible 19:33:30 Has it had a `ci_complete` run? Can't remember if those would trip after the fact or not, but we don't want to break it in ansible/ansible if so 19:33:53 (or if change detection would exclude anything for those tests anyway) 19:34:24 I think change detection runs everything if something in test/lib changes (or maybe it's even more clever, depending on what changed in there) 19:34:49 So long as we're pretty sure it's not going to immediately bomb devel, I'm ok to merge 19:35:40 it shouldn't affect devel anyway since it doesn't do any checks for ansible.builtin 19:36:02 ah, I missed that part 19:36:19 OK, any objections to merge? Speak now or moan later ;) 19:36:20 ansible.builtin versions are ansible versions, and they don't stick to semver ;) 19:36:55 not yet 19:37:03 yep. from 3.0.0 on hopefully. 19:37:09 1.0.0 19:37:18 Actually lemme see if mattclay will give it a quick once-over before it gets merged, but sounds like nobody's against it... 19:37:20 since ansible is no more, we have core and community 19:37:28 nitzmahone: thanks! 19:37:36 bcoca: if it's renamed, yeah, fine :) 19:39:47 I approved, but I've asked for a quick once-over from Matt C, and he'll merge it if it looks OK to him 19:40:07 anything else? 19:40:15 not I 19:40:37 wasnt asking you ;-p 19:40:46 :P 19:40:48 for this topic, or for other topics? 19:41:31 both 19:44:21 maybe about https://github.com/ansible/ansible/pull/71734: does that need more discussion from core POV? 19:45:29 so --type would filter sanity to only the matching files? 19:46:06 you dont need to add author: unkown, that is the default 19:46:10 Seems like renaming/aliasing the test to `validate-plugins` would probably be a Good Thing, but I suspect there be dragons 19:46:19 alias it? 19:46:20 (esp with ignores/skips) 19:46:31 also examples can be other formats (non yaml) 19:47:02 jtanner: do you mean --plugin-type ? 19:47:09 yeah 19:47:56 jtanner: it assumes that all paths passed are plugins of that type 19:48:19 wait .. i wrote url lookup? 19:48:30 * bcoca now needs to do another good thing to level out karma 19:48:42 nitzmahone: yeah, that will probably be a hard one 19:48:52 bcoca: i.e. you want to make `author:` optional? 19:49:11 it is/was 19:49:25 specially when we offloaded most of its function to botmeta 19:49:40 and copyright notice requires its own entry, very little reason to have 'author' field anymore 19:49:41 from https://github.com/ansible/ansible/issues/71794 I understood that it should be required 19:50:16 i.e. same behavior as for modules 19:50:24 not useful in modules either anymore 19:50:46 i agree about 'same enforrcement' across the board, i would say 'optional' should be that enforcement 19:51:03 why isn't it useful in modules? 19:51:25 there's no other way to show it in the docs 19:51:37 (well, no other way without forcing all collections to use BOTMETA or stuff like that) 19:52:14 Yeah, I wouldn't say it's not useful, though its value is often stale 19:52:32 you mean `author:` or BOTMETA? 19:52:34 author was mostly a way to be able to contact maintainer 19:52:45 (and it's an "attractive nuisance" for people adding themselves to the author list for very minimal changes) 19:52:47 modules already should have 'auythors' in the copyright notice 19:52:59 now we can argue, we should show it also on ansible-doc 19:53:17 also many authors dont want to put info in docs, so why i think it shoudl be optional 19:54:00 so should it be optional for both modules and plugins? 19:54:12 imho, yes 19:54:27 that's probably something that should be voted on 19:54:32 we 'required' it long before botmeta appeared and made it less useful 19:54:59 in that case, why still require a strict form which disallows many things, such as "Ansible Network Team"? 19:55:27 i wouldnt 19:55:38 again, many of those 'requriements' are ancient and not used anymore 19:57:46 hmm, I guess that's something core team should discuss internally, or should be discussed here 19:58:27 in practice we stopped using author long ago, making it mandatory at this point, seems silly 19:58:40 I'd be fine with making it optional for modules. I can't remember who was in triage that day, but I think we were mostly interested in consistency, and we didn't discuss the fact that `author` isn't really useful now. 19:59:12 shertel: i think your first year was last one in which we used it as our main info source 19:59:18 * nitzmahone -> WWG 19:59:31 right now, iirc its 'fallback' for bot when botmeta is empty (and that is fine) 19:59:41 * shertel nods 20:00:50 collections don't necessarily have a botmeta, but they can enforce their own rules? 20:01:35 honestly I'm not sure that "author" was intended to be useful, but more for marketing ones name 20:02:07 sivel: which why i think we can keep as optional, forceing them to market name seems ... idk .. not a 'marketing' issue 20:02:28 just providing my unsolicited feedback ;) 20:02:31 not saying we remove author, but main reason we enforced it in modules was to be able to find 'maintainer' 20:02:35 while I wait on eternal hold 20:03:55 i can't remember if authors ever showed up in docs 20:04:01 it does 20:04:07 it used to not to 20:04:14 but ifixed that long time ago 20:04:16 they weren't formatted consistently until i tried using them for ansibot 20:04:26 yes, that is when we added rules 20:04:31 then you moved to botmeta 20:04:45 and botmeta then pretty much deprecated the need to use authors 20:04:49 yeah 20:04:51 exactly 20:04:57 and now we basically don't use botmeta 20:04:59 :) 20:05:02 he 20:05:05 =) 20:05:16 goes to my stance, keep field, but leave it optional 20:05:33 wfm 20:05:40 +1 20:05:48 how about its form? should it be an arbitrary list of strings or string? 20:05:51 there were never any good rules to define what constituted "authorship" 20:05:53 (also for modules?) 20:06:02 if community wants to enforce something for the collections they accept, that is diff issue 20:06:28 felixfontein: its a list of strings, right now we expect specific format (since ethat is how we did github cc or irc pings, but that is moot now) 20:06:54 jtanner: mostly 'who do we nag on open tickets on this?' 20:07:12 bcoca: I know, I'm trying to find out what we want in the future :) 20:07:24 sometimes. some people added themselves after changing a single line but didn't want to be nagged 20:08:26 yep, seen that... 20:08:27 i always felt author should be deprecated by botmeta and if people wanted to know who contributed something to check the git log 20:08:36 jtanner: that was 'market me, but dont bug me' 20:08:50 jtanner: we also have copyright notice in file ... 20:10:32 k, with that im closign meeting 20:10:36 #endmeeting