Skip to content
This repository has been archived by the owner on Jun 10, 2024. It is now read-only.

Public Core Meeting Meeting - April, May 2017 #159

Closed
gundalow opened this issue Apr 4, 2017 · 46 comments
Closed

Public Core Meeting Meeting - April, May 2017 #159

gundalow opened this issue Apr 4, 2017 · 46 comments

Comments

@gundalow
Copy link
Contributor

gundalow commented Apr 4, 2017

Please leave a comment regarding any agenda item you wish to discuss. If you don't show up for the meeting, your item will be skipped.

If your IRC nick is different from your Github username, leave that as well.

See https://github.com/ansible/community/blob/master/MEETINGS.md for the schedule

Once an item has been addressed it should get strike-though ~~strike-though~~

@gundalow
Copy link
Contributor Author

gundalow commented Apr 4, 2017

Discuss extending the module "shipit" workflow to non-modules, such as:

  • lib/ansible/plugins
  • lib/ansible/module_utils
  • contrib/inventory

Migrated from

Update 12 Jan 2017 Meeting

  • All approved. We'll start with contrib/inventory as most of those are community maintained as it is.
    • Will come up with a list of dynamic inventory proposed metadata for next week.
    • bcoca created spreadsheet; will update us on status.

Update 9th Feb 2017 Meeting
For contrib/inventory

  • Spreadsheet exists
  • bcoca to speak with @abadger to find out how we add python metadata to config files
  • bcoca to organise meeting (internally?) to vote and get metadata agreed

Feb 21 Meeting

  • For dyn inventory config files, abadger proposes that the config files use the asociated script's metadata.
    • Note: There are both yml and ini config files. Need to make sure both are handled by the metadata code.
    • ryansb will rename the one outlier, consul.ini to consul_io.ini to match with the script consul_io.py. (code will look for both file names for backwards compat) Completed

Mar 2 Meeting

  • Nothing new to discuss while we wait on module metadata to be reworked.

Mar 14

  • Metadata rework is done.

2017-03-23

  • jtanner to evaluate and write bot workflow for non-module shipits
  • tracked in implement shipit workflow for !modules ansibullbot#437
  • need more info on what 'rework is done' means
    • Metadata rework done means that the final draft of metadata 1.0 was accepted and the modules have that form of the metadata.
      2017-04-18
  • jtanner, are there any other decisions that need to be discussed here? Or are we at the point where everything is approved just needs to be implemented?

@gundalow
Copy link
Contributor Author

gundalow commented Apr 4, 2017

From luto: ansible/ansible#19297 Fix for wildcards inside of a path for fileglob lookup (ie: with_fileglob: "/tmp/*/some.conf")

Note this is for stable-2.2 and stable-2.3

Migrated from #150 (comment)

  • 24 Jan Meeting: bcoca had two observations which need to be considered as the code is reviewed:
    • (1) bcoca: dwim code always needs careful consideration because it can break a lot of things and isn't always obvious why it's doing things.
      • Submitter (rupran) says: "what it basically does is factor out a part of path_dwim_relative_stack (leaving the functionality untouched) to use it in with_fileglob"
    • (2) bcoca: lookups should use same function, once you start diverting them they become unpredictable but just on consistency with lookups, not happy to duplicate and modify functionality
  • 9 Feb: @bcoca to review
  • 21 Feb: Still on @bcoca's radar to review
  • 2 Mar: Still on @bcoca's radar to review
  • 7 Mar: Recently rebased. Still on bcoca's radar.
  • 4 Apr: Alternative fix from bcoca [WIP] Allow fileglob of dirs ansible#23265
  • 11 Apr: Lots of options on how this should work, @abadger to enumerate and revisit in future meeting
  • enumeration and pros/cons of alternatives: https://gist.github.com/abadger/7f9466272d84fad9ab7c4207a65a67fd
  • 18 Apr: @abadger and @bcoca discussed and think that there's pretty big cornercases for all the strategies except erroring. They agree that we can handle wildcards in absolute paths but not relative paths. So bcoca will update the PR to error in that case.

@gundalow
Copy link
Contributor Author

gundalow commented Apr 4, 2017

Define and document proposals process i.e ansible/proposals#50 was created and 'agreed' to immediately, w/o giving time for community feedback, which is happening now after implementation.

Migrated from #150 (comment)

We might need to review/update this:
https://github.com/ansible/proposals/blob/master/proposals_process_proposal.md

7 Feb Meeting

Discussed at the meeting. Many problems with the meta-proposal for creating proposals were discussed. Minimum timeframes were proposed but no agreement could be come to. Proposal documenting the new proposal process was asked for.

14 Feb Meeting

  • ACTION: allanice001 has kindly offered to raise a PR to update proposals_process_proposal.md
  • In the future we can look at moving this under docs.ansible.com/ansible/NEWDIR/process_proposal.html , Details about our release process, proposal process, etc will go there, rather than under dev_guide. Need to bikeshed a name. This is something for after Ansible 2.3 is released.

2 Mar Meeting

  • No updates from allanice001. Still waiting on PR to update proposal.

2017-03-23
reviewed: ansible/proposals#59
no consensus, some feedback (see meeting notes) to be posted on ticket.

20174-04

@gundalow
Copy link
Contributor Author

gundalow commented Apr 4, 2017

from @bcoca How should paths work? https://github.com/ansible/ansible/pull/22546

2017-04-04

  • Not just a bugfix, it's setting expectations on how pathing works
  • tkuratomi thinks that relative paths have too much ambiguity and would not expect many of the paths to be relative to the config file. Would rather have relative paths that cannot be made relative to playbook error.
  • bcoca and tkuratomi both voiced tentative support for alikins' idea of having placeholders for relative to CONFIG, CWD, etc that could be used.
    2017-04-18
  • Deferred for now (ansible config may interfere with prototyping). To be put back on the agernda when we're ready to code/make a decision.

@gundalow
Copy link
Contributor Author

gundalow commented Apr 4, 2017

expand doc fields
ansible/proposals#58

2017-03-21:
types +1 as long as they are strict list
config: @bcoca to provide more examples as it seems to overlap with existing options

2017-03-23:
new proposal seems close to consensus, some details to discuss:

  • priorities
  • shared docs (fragments?)
  • field naming (bikeshed away)

Ticket is open to updates/discussion and once those are done it will be brought back for final approval

2017-04-11:

  • abadger/bcoca/nitzmahone are OK with proposal as-is, ready for implementation?
    2017-04-18
  • No objections/further comments have been raised this week.. Asked at meeting if anyone else had something to say and nothing was added. Calling this approved. Go forth and implement!

@gundalow
Copy link
Contributor Author

gundalow commented Apr 4, 2017

From bcoca

Now that we support docs for many more plugin types, should we require all new plugins of those types to have docs included?

2017-03-30

  • All new plugins should have docs
  • Should be enforced by CI
  • We should not just add placeholder docs to existing to make them pass - will be difficult to know what requires work.
  • Therefore CI requires skip-list

2017-04-18

  • Note: not all plugins support docs yet. Use andible-doc -h to figure out which types of plugins need to be tested

  • This seems to be approved once CI can enforce it. Leaving this on the agenda until we know mattclay is aware of it, then can remove.

@gundalow
Copy link
Contributor Author

gundalow commented Apr 4, 2017

From @mattclay

ansible/ansible#9563 (Adding nested dictionary lookups) - Do we want to accept this PR?

NOTE: Tests were passing until more strict checks were added recently. Docs will need to be updated to reference the 2.4 release instead of 2.2.

@gundalow
Copy link
Contributor Author

gundalow commented Apr 4, 2017

~~From @bmildren ~~

Hi all, I'm trying to find how to progress this PR past community review, and it was suggested I add it here as an item to discuss.. ansible/ansible#19872

(my irc handle is productiondba)

2017-04-04 Merged

@gundalow
Copy link
Contributor Author

Ansible 2.3.0 FINAL has been released. Thank you for all your help with this release
https://groups.google.com/forum/#!topic/ansible-project/aKnggI-Y8dk

@nitzmahone
Copy link
Member

nitzmahone commented Apr 18, 2017

Revisit frequency/timing of core meetings- participation and efficacy feels pretty low lately.

Decided on keeping current schedule for now.

@gundalow
Copy link
Contributor Author

gundalow commented May 2, 2017

Module option validation. passed on to @privateip as shown below

I've recently seen a few modules that:

  • Ensure an int is within a specific range
  • Ensure a option is an IPv4 (or IPv6) address

Should class AnsibleModule in basic.py be updated to support these?

201705-02

@azaghal
Copy link

azaghal commented May 4, 2017

aka branko on IRC

I would like to get some feedback for a dconf module. Since I have some obligations on next Tuesday, if this could be discussed during meeting on Thursday, 2017-05-11, that would be great.

https://meetbot.fedoraproject.org/ansible-meeting/2017-05-11/ansible_core_meeting.2017-05-11-15.00.log.html#l-108

https://meetbot.fedoraproject.org/ansible-meeting/2017-05-11/ansible_core_meeting.2017-05-11-15.00.log.html#l-109

Before I continue making changes to the proposed pull request, I would like to get feedback on what would be acceptable in order to merge it so I would not spend effort going into wrong direction.

Problem is as follows:

  • dconf is configuration database used by a lot of Gnome and/or GTK+ applications. A module that wraps around it makes it possible to automate provisioning of configuration for such applications.
  • In order to change settings via dconf, it is necessary to have a running D-Bus user session. Session is provided by running dbus-daemon binary.
  • In order to ensure integrity and avoid accidental overwrites of settings, one should not run multiple D-Bus user sessions.
  • Connection to D-Bus session is specified via environment variable DBUS_SESSION_BUS_ADDRESS.
  • Detecting a running D-Bus user session address is a non-trivial matter due to lack of standardisation in D-Bus itself, and across distributions. If you are running systemd, the address should be predictable. If running something else, not so much. For example, on Gentoo this is a random string.
  • D-Bus user session (dbus-daemon) can be launched in different ways based on D-Bus version present. If you're lucky (running D-Bus 1.8.0+), you can use dbus-run-session to wrap your command, and it will kill off the dbus-daemon once your command exits. If not, you need to launch the daemon and make sure to kill it off after you're done.
  • Current implementation in the pull request will try its best to find a running session, and if that fails it will run its own. Code will also clean-up after itself as needed, supporting both the dbus-run-session and spawning of its own daemon.
  • Problem is that this means the dconf module is behaving like a process manager, at least for D-Bus user session. This might not be desirable behaviour for publicly available Ansible module. As pointed out on IRC by sivel, a module should not manage daemon processes.
  • Alternative would be to rely and require user to take care of spawning D-Bus user session and providing the address. However, this reduces usability, and pushes burden with all the listed things onto module user (and good luck someone would read up on how D-Bus daemon is spawned and killed - I know I didn't until somebody told me in the pull request review :).

@alikins
Copy link

alikins commented May 9, 2017

@gundalow
Copy link
Contributor Author

Ansible 2.2.3 FINAL released, 2.1.6rc1 and 2.3.1rc1 available for testing

All of these releases address at least one CVE (listed below). Users of 2.2.x should upgrade to 2.2.3 as soon as possible. The CVE impacting 2.1.x and 2.3.0 is MODERATE in nature, however users should look to upgrade as soon as the final releases are done.
https://groups.google.com/forum/#!topic/ansible-devel/etlpnJDerY0

@thaumos
Copy link

thaumos commented May 11, 2017

ansible/ansible#24463 MERGED

@azaghal
Copy link

azaghal commented May 14, 2017

aka branko on IRC

Would like to discuss ansible/ansible#23015 once again (dconf module) since in the current form module might be a bit useless even to me.

action : https://meetbot.fedoraproject.org/ansible-meeting/2017-05-16/ansible_core_meeting.2017-05-16-19.00.log.html#l-76

@gundalow
Copy link
Contributor Author

gundalow commented May 15, 2017

~~Is DOCUMENTATION = r''' the correct way"? If so we need to update our docs. Ditto. For RETURNS and EXAMPLES. ~~

action: https://meetbot.fedoraproject.org/ansible-meeting/2017-05-16/ansible_core_meeting.2017-05-16-19.00.log.html#l-170

answer: depends on context

@thaumos
Copy link

thaumos commented May 16, 2017

@gundalow
Copy link
Contributor Author

gundalow commented May 17, 2017

Ansible Contributor Summit 4 (Part of AnsibleFest 2017 London)

We are happy to announce our fourth Ansible Contributors Summit!
AnsibleFest London (http://www.ansible.com/ansiblefest) will be on Thursday 22nd June, and the Contributor Summit will be held on Wednesday 21st June, from 9:30am to 4pm (time subject to change).

Contributors Summit is a day-long working session with the core team and key contributors to discuss important issues affecting the Ansible community. You can participate in person or online

Proposed topics: https://public.etherpad-mozilla.org/p/ansible-summit-june-2017

See https://groups.google.com/forum/#!topic/ansible-devel/stUITiM6hMs for more info

action: noted

@thaumos
Copy link

thaumos commented May 18, 2017

ansible/ansible#24751

Merged

@thaumos thaumos changed the title Public Core Meeting Meeting - April 2017 Public Core Meeting Meeting - April, May 2017 May 23, 2017
@Akasurde
Copy link
Member

Akasurde commented May 24, 2017

Need suggestion about Yum module - ansible/ansible #24822

note: tested with downgrade + upgrade
action: integration tests to be written

@bcoca
Copy link
Member

bcoca commented May 24, 2017

vote on ansible/ansible#24867

#note previous vote is thought to have been to reject PR
#action decide on action for PR and provide sufficient reasoning
#action dag to remove cosmetic changes and abadger+jtanner +1 on merge

@bcoca
Copy link
Member

bcoca commented May 24, 2017

do we allow 'none/null' values on required parameters?

ansible/ansible#24871

#action vote results were "yes" at last meeting
#note vote results were "yes" at last meeting

@willthames
Copy link
Contributor

willthames commented May 26, 2017

`when:` conditions are warning even for variables composed of other variables - this is most unexpected.

ansible/ansible#25053

I almost certainly won't be able to make the meetings (1am and 5am my time) but @bcoca is aware of my thoughts.

My helpful proposal that should catch 99% of problems but not cause issues for lots of users (i.e. it should be an improvement, but not necessarily perfect) is:

'{{' in condition and '}}' in condition or '{%' in condition and '%}' in condition

action: need to revisit once sivel can attend and discuss complete removal of warning

@mikedlr
Copy link
Contributor

mikedlr commented May 26, 2017

The apt package installing module is a bit inefficient and could do with proper refactoring. There was a new patch to handle multi-architecture. I tried to make a unit test for it and it turned up a few of obvious deficiencies. For example the following:

  • there's a huge section of code hidden inside a try which makes exception handling very unpredictable
  • the dpkg installation function is too big and complex
  • every .deb which is examined requires three external calls to dpkg (in turn calling dpkg-deb) and so it's needlessly inefficient and complex.

I've made more detailed comments in my review in the pull request for the apt module.

This is a core maintained module so it would be good if someone could look at it and see if they'd be interested in taking on fixing it. @evgeni who wrote the patch is willing to help but doesn't think he can drive this. I can help with making the unit tests but don't really know the module or debian packaging so well. @gundalow suggested taking this up in the core meeting and will be willing to talk about it if I'm not available.

action: mentioned PR is merged
action: mikedlr to open new bugs on refactoring so we can mark needs_contributor

@willthames
Copy link
Contributor

willthames commented May 30, 2017

There are a lot of curated modules in the aws space. I wonder how many of them the committer team are really willing to support - should we make some/most/all of them community managed?
_ec2_vpc.py
aws_kms.py
cloudformation.py
ec2.py
ec2_ami.py
ec2_asg.py
ec2_eip.py
ec2_elb.py
ec2_elb_lb.py
ec2_facts.py
ec2_group.py
ec2_key.py
ec2_lc.py
ec2_metric_alarm.py
ec2_remote_facts.py
ec2_scaling_policy.py
ec2_tag.py
ec2_vol.py
ec2_vpc_dhcp_options.py
ec2_vpc_igw.py
ec2_vpc_nacl.py
ec2_vpc_nacl_facts.py
ec2_vpc_net.py
ec2_vpc_net_facts.py
ec2_vpc_peer.py
ec2_vpc_route_table.py
ec2_vpc_subnet.py
ec2_vpc_subnet_facts.py
ec2_vpc_vgw.py
efs.py
efs_facts.py
iam.py
s3.py
s3_bucket.py
s3_lifecycle.py
s3_logging.py
sns_topic.py
sqs_queue.py
sts_assume_role.py
sts_session_token.py

#action willthames committer rights to be discussed internally
#action aws maintainers to group together and communicate

@willthames
Copy link
Contributor

What constitutes authorship for a module? What criteria should there be to be added to the authors list? What responsibilities (if any) does being added to the authors list bring?

@chriskarel
Copy link

I'd like to bring up PR #24580 ansible/ansible#24580

I believe the PR is complete, and even has some additional unit tests. But it hasn't been approved enough to get included. One of the module maintainers recommended bringing it up in this meeting.

@gundalow
Copy link
Contributor Author

gundalow commented Jun 1, 2017

ansible/proposals#65 Allow documenting option deprecation

@erasmix
Copy link

erasmix commented Jun 1, 2017

I would like to discuss PR#21857 and PR#21764 on the next Core Meeting

@pilou-
Copy link
Contributor

pilou- commented Jun 6, 2017

I would like to request reviews for PR#23862.

@dagwieers
Copy link
Contributor

We need guidance on ordered dictionaries for Windows modules. I prefer a common solution. ansible/ansible#25265 (comment)

@dagwieers
Copy link
Contributor

Would like to discuss:

@cyberark-bizdev
Copy link

cyberark-bizdev commented Jun 8, 2017

I would like to discuss PR#21857 and PR#21764

@erasmix will be attending
@cyberark-bizdev will be in standby for discussion

@azaghal
Copy link

azaghal commented Jun 8, 2017

@thaumos
Copy link

thaumos commented Jun 8, 2017

Please use #162 going forward.

@ansible ansible locked and limited conversation to collaborators Jun 8, 2017
@gundalow
Copy link
Contributor Author

Rolled over to June's agenda, please use #162

@dagwieers dagwieers added core and removed core labels Jun 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests