20:00:02 <jborean93> #startmeeting Ansible Windows Working Group
20:00:02 <zodbot> Meeting started Tue Mar 27 20:00:02 2018 UTC.  The chair is jborean93. Information about MeetBot at http://wiki.debian.org/MeetBot.
20:00:02 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
20:00:02 <zodbot> The meeting name has been set to 'ansible_windows_working_group'
20:00:14 <jhawkesworth_> hey
20:00:17 <jborean93> hey
20:00:19 <jborean93> again :)
20:00:24 <jhawkesworth_> :-)
20:00:25 <jborean93> #chair jhawkesworth_
20:00:25 <zodbot> Current chairs: jborean93 jhawkesworth_
20:00:35 <jborean93> nitz is on PTO so won't be here
20:01:00 <jhawkesworth_> oh ok, hope he's having a good break
20:01:40 <jhawkesworth_> timezone change means kids in bed, hopefully I can be around a bit more now
20:01:41 <jborean93> I think he's traveling to the lone star state
20:02:07 <jborean93> that's good to hear
20:02:18 <jborean93> DST isn't a think where I am
20:02:26 <jborean93> will see if anyone else joins in before starting
20:02:35 <jhawkesworth_> yeah, give it a minute
20:02:36 <jborean93> s/think/thing
20:05:09 <jborean93> looks like it is just us 2 today
20:05:28 <jborean93> #topic win_dsc return params https://github.com/ansible/community/issues/294#issuecomment-376009166
20:05:40 <jhawkesworth_> yeah - i guess holiday season starting.
20:05:51 <jborean93> I'll probably bring this is up at the next meeting when more people are here but what are your thoughts on this
20:06:01 * jhawkesworth_ reading through the bug report
20:08:20 <jborean93> The return values can get quite large, especially when dealing with dicts as DSC doesn't take in normal hashtables but CimInstances
20:08:44 <jhawkesworth_> yeah that CimInstance example is a monster.
20:09:22 <jhawkesworth_> I think I favour dag's suggestion of something like an 'output_level' param for returning the parsed params
20:09:40 <dag> o/
20:09:45 <jborean93> #chair dag
20:09:45 <zodbot> Current chairs: dag jborean93 jhawkesworth_
20:09:46 <jborean93> hey
20:09:50 <jhawkesworth_> hey dag
20:11:07 <jborean93> even so, the only main differences you may get are seeing strings as ints and vice versa
20:11:24 <jborean93> we don't do any special handling for strings to arrays like splitting by `,`
20:13:51 <jborean93> plus the dict to CimInstance can really clog the output
20:13:51 <jhawkesworth_> I'm not actually using win_dsc module so I'm only thinking of it from the point of view of what fits in with rest of ansible / what is least surprising change to it
20:14:32 <jhawkesworth_> I wondered for a minute if it could be something that would be done only in check mode... but I don't like that idea
20:14:34 * jborean93 wonder's if we should add an option that is to export the definition to a file which people can manually run later on
20:16:40 <jhawkesworth_> hmm, writing to a file might be nice for development / test purposes perhaps
20:17:00 <jborean93> yea, probably little gain
20:17:13 <jborean93> I just develop locally and use a debugger if necessary
20:17:44 <jborean93> that issue in question was a bit hard to find out what was wrong as the DSC exception it threw wasn't helpful at all
20:17:56 <jborean93> Having the parsed params would not have made that easier
20:18:03 <jhawkesworth_> ... but the extra code would be there for every invocation
20:20:13 <jhawkesworth_> I see ... handling unhelpful error messages is going to be perenial issue for ansible.
20:20:50 <jborean93> yep, still need to fix that part
20:21:21 <jborean93> but for this win_dsc side, the unhelpful exception message was being returned, it was just unhelpful to begin with as the DSC engine didn't give up any good info
20:22:15 <jborean93> ok so sounds to me we don't want to add the parsed values back in
20:22:32 <jhawkesworth_> not as default behaviour, for sure.
20:22:49 <jborean93> it would be antithesis to what we are trying to move onto
20:23:30 <jborean93> when the invocation args are finally added back in that would be quite helpful but that's not covered in the module
20:24:06 <jhawkesworth_> If it doesn't actually help you discover real underlying issues, there seems little point
20:24:28 <jborean93> sounds good
20:24:43 <jborean93> moving on, unless someone wanted to add more
20:24:47 <jborean93> #topic https://github.com/ansible/community/issues/294#issuecomment-376567904
20:25:10 <jborean93> I haven't fully reviewed the PR but wondering whether we should be adding the alternative shebang to the sanity module
20:25:21 <jhawkesworth_> yeah that's ugly.
20:25:32 <jborean93> Why was that needed?
20:25:57 <jhawkesworth_> test fails if it has \r line ending
20:26:34 <jhawkesworth_> assumption is there is 1:1 relationship between file extension and valid shebang
20:27:14 <jborean93> by tests do you mean the sanity check. I would have thought we add the test module to ensure the code changes are good but we don't change the requirement for modules to be included in Ansible itself
20:27:59 <jhawkesworth_> hmm, yeah the code as it stands would allow shebang \r in core modules.
20:28:28 <jhawkesworth_> but line endings check would still fail it if there was a \r on any line in core module
20:28:33 <jhawkesworth_> so it would get noticed
20:28:34 <jborean93> I think I'm fine with the overall CI tests failing for that PR as long as it's just a failure saying the test modules have bad line ending
20:29:45 <jhawkesworth_> I'd prefer to code something that expresses what is and isn't allowable
20:29:57 <jhawkesworth_> open to suggestions on how
20:30:42 <jhawkesworth_> maybe the way to handle this is just to add a note in the porting guide that says \r is no longer acceptable in .ps1 files (change from 2.4)
20:31:35 <jborean93> When you say `\r` do you mean `\r\n`?
20:31:36 <jhawkesworth_> actual change I've made is a one liner, all the rest of the changes in the PR are a consequence of allowing the change.
20:32:01 <jborean93> I think we want to only allow `\n` for modules within Ansible
20:32:17 <jborean93> But change the internal code to handle `\r\n` as because have custom modules in that format
20:32:18 <jhawkesworth_> yeah but in most of the cases the \n is taken care of
20:33:02 <jborean93> wondering whether it should just be `module_data.startswith("#!powershell")` isntead of just comparing the whole line
20:33:10 <jborean93> that would work in both cases
20:33:22 <jhawkesworth_> yeah agreed, core modules should be free of \r\n line endings - its custom modules that the PR is about
20:33:39 <jhawkesworth_> I think there was a recent change to make it check the whole line
20:33:49 <jhawkesworth_> because some modules had something like
20:34:00 <jhawkesworth_> #/bin/python -tt
20:34:22 <jhawkesworth_> and the startswith check was ignoring the '-tt' bit
20:35:43 <jborean93> ok, I just think we should remove the code-smell change for the shebang check
20:35:55 <jborean93> The rest of the code I'm find with (just need to test it)
20:38:28 <jhawkesworth_> so .. just no change to test/sanity/code-smell/shebang.py ?
20:39:31 <jborean93> I think so
20:39:46 <jborean93> it doesn't seem like it is needed as we want to have that sanity check for modules to be included in Ansible
20:41:26 <jhawkesworth_> I see.  how about I add a skip list for modules and just add the 2 modules in the integration tests
20:42:16 <jborean93> I thought you already skipped those test modules?
20:42:55 <jhawkesworth_> skipped in the line endings test, not in the shebang test
20:43:07 <jborean93> ah ok
20:43:16 <jborean93> then yes, skip only those 2 test modules
20:43:30 <jborean93> there should be an ignore file or some list somewhere
20:43:36 <jborean93> mattclay would know more
20:43:57 <jhawkesworth_> great, that's better than alternative_shebangs - which is too general.  Thanks for helping me think that through
20:44:09 <jborean93> that's ok
20:45:19 <jborean93> cool last topic
20:45:28 <jborean93> #topic win_pester https://github.com/ansible/community/issues/294#issuecomment-376568860
20:45:46 <jborean93> I haven't looked at the PR yet
20:46:12 <jborean93> my only concern is that it's custom tailored towards the author's workflow and isn't generic but that's without even seeing the code itself
20:47:40 <jhawkesworth_> yeah it just occurred to me that if ansible might want to make some use of pester internally, having a community module might complicate matters.
20:48:18 <jborean93> I think the plan for using pester internally was to use it in the docker container we run the pslint sanity checks on
20:48:35 <jborean93> would restrict things a bit to powershell core cmdlets only but no need to manage a separate Windows host
20:48:58 <jborean93> nitzmahone was looking into that and I believe that was what he was planning to do
20:49:16 <jborean93> but it is definitely an option to use that module if it fits in with what we need
20:50:02 <jhawkesworth_> sounds like it would  be not a great idea to make a decision about it until nitzmahone has had a look
20:50:50 <jborean93> yep
20:51:02 <jhawkesworth_> ok, lets park it for now then
20:51:15 <jborean93> sounds good
20:51:20 <jborean93> #topic open floor
20:51:27 <jborean93> anything else we wanted to talk about?
20:52:02 <jhawkesworth_> not from me tonight
20:52:46 <jborean93> cool
20:52:59 <jborean93> well unless dag has anything he wants to add I'll bring this to a close
20:54:34 <jborean93> silence is golden
20:54:38 <jborean93> thanks everyone
20:54:40 <jborean93> #endmeeting