New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated win_disk_facts module #37798

Open
wants to merge 3 commits into
base: devel
from

Conversation

Projects
None yet
5 participants
@marqelme
Contributor

marqelme commented Mar 22, 2018

SUMMARY
  • Added trimmed option to module win_disk_facts
  • Removed required administrator privileges

The trimmed option can be used to reduce the output of the module.
Then only the disk fact details will be displayed but no detailed information about the partitions and volumes of the disk.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

win_disk_facts

ANSIBLE VERSION
ansible 2.4.0.0
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/dist-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.12 (default, Nov 20 2017, 18:23:56) [GCC 5.4.0 20160609]

marqelme added some commits Mar 22, 2018

@ansibot

This comment was marked as resolved.

Contributor

ansibot commented Mar 22, 2018

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/modules/windows/win_disk_facts.py:28:19: W291 trailing whitespace

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/windows/win_disk_facts.py:0:0: E309 version_added for new option (trimmed) should be 2.6. Currently 0.0

click here for bot help

@ansibot

This comment has been minimized.

@dagwieers

This comment has been minimized.

Member

dagwieers commented Mar 22, 2018

This PR is undoing a lot of things we recently added for the v2.5 release.

I am also not convinced we want an option "trimmed" here. This requires a discussion, both for the reasons as well as what would be an acceptable interface.

- name: Output second disk serial number
- name: get disk facts
win_disk_facts:
- name: output second disk serial number

This comment has been minimized.

@dagwieers

dagwieers Mar 22, 2018

Member

This is undoing the example.

returned: if disks were found
type: int
sample: 3
disks:

This comment has been minimized.

@dagwieers

dagwieers Mar 22, 2018

Member

This is undoing the explicit ansible_disk key.

type: int
sample: 227727638528
type: string
sample: "100GiB"

This comment has been minimized.

@dagwieers

dagwieers Mar 22, 2018

Member

We wanted raw values on purpose.

@ansibot ansibot removed the needs_triage label Mar 22, 2018

@ansibot ansibot removed the ci_verified label Mar 23, 2018

@marqelme

This comment has been minimized.

Contributor

marqelme commented Mar 23, 2018

@dagwieers
Yes, this was a mistake, I have corrected it.

Sure, please discuss this, just thought this option would help because the module creates a lot of output if you have a lot disks, partitions and volumes.

@ansibot

This comment was marked as resolved.

Contributor

ansibot commented Mar 23, 2018

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/windows/win_disk_facts.py:0:0: E309 version_added for new option (trimmed) should be 2.6. Currently 0.0

click here for bot help

@ansibot ansibot added the ci_verified label Mar 23, 2018

@ansibot ansibot added the stale_ci label Mar 31, 2018

@chopraaa

This comment has been minimized.

Contributor

chopraaa commented Apr 21, 2018

Is there a reason we're not treating volumes and disks differently? I thought we arrived at a conclusion over at #27634 that it's better to create different modules.

IMHO this is way too bloated and all I'm all against the disk_facts module. :/

@dagwieers

This comment has been minimized.

Member

dagwieers commented May 14, 2018

Beware that the deadline for getting new features/modules accepted in Ansible v2.6 is nearing, it is set to either 2018-05-17 or 2018-05-25. If you are blocked, or you need feedback, please discuss on IRC channel #ansible-windows or add a comment to the Windows Working Group meeting agenda to get it resolved.

@dagwieers

This comment has been minimized.

Member

dagwieers commented Aug 16, 2018

Beware that the deadline for getting new features/modules accepted in Ansible v2.7 is nearing, it is set to either 2018-08-23 or 2018-08-30. If you are blocked, or you need feedback, please discuss on IRC channel #ansible-windows or add a comment to the Windows Working Group meeting agenda to get it resolved.

@taschaal

This comment has been minimized.

taschaal commented Nov 5, 2018

I don't see it mentioned anywhere, but this module doesn't appear to work with windows dynamic disks. Am I correct in that?

@marqelme

This comment has been minimized.

Contributor

marqelme commented Nov 5, 2018

@ansibot ansibot added the owner_pr label Nov 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment