Skip to content
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

big revamp on xfconf, adding array values #62578

Closed
wants to merge 6 commits into from
Closed

big revamp on xfconf, adding array values #62578

wants to merge 6 commits into from

Conversation

russoz
Copy link
Contributor

@russoz russoz commented Sep 19, 2019

SUMMARY

Adds arrays support for xfconf module. There is another PR already open adding that feature (#50108). However, in that PR, the maintainer of the xfconf module asked if anyone is interested in taking over the module (#50108 (comment)).
So, I have contacted him and I am creating this PR after his instructions.

Fixes #46308

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

xfconf

ADDITIONAL INFORMATION

This is largely based on the previous PR, so the examples found there apply. On top of that, it handles a couple of other cases. See below:

  - name: Configure single workspace
    xfconf:
      channel: xfwm4
      property: "{{ item.property }}"
      value_type: "{{ item.type }}"
      value: "{{ item.value }}"
      state: present
    with_items:
    - { property: /general/workspace_count, type: int, value: 1 }
    - { property: /general/workspace_names, type: string, value: Main }
      # ^ assumed as an array because the existing property is an array

  - name: Configure four workspaces
    xfconf:
      channel: xfwm4
      property: "{{ item.property }}"
      value_type: "{{ item.type }}"
      value: "{{ item.value }}"
      state: present
    with_items:
    - { property: /general/workspace_count, type: int, value: 4 }
    - { property: /general/workspace_names, type: string, value: ['Main', 'Work1', 'Work2', 'Tmp'] }
      # ^ assumed as an array because length of "value" is > 1
      # ^ type can be a single element, which is then used on all the elements of "value"

  - name: New property forced to be an array (dummy example)
    xfconf:
      channel: xfwm4
      property: /my/new/one_element_array
      value_type: string
      value: potatoes
      force_array: yes
      state: present
      # ^ assumed as an array only because "force_array" is yes

  - name: Array with mixed types (dummy example)
    xfconf:
      channel: xfwm4
      property: /my/mixed_array
      value_type: [ int, string ]
      value: [ 42, potatoes ]
      state: present
      # for mixed-type arrays the value type must list the type for each item

@ansibot
Copy link
Contributor

ansibot commented Sep 19, 2019

@ansibot ansibot added affects_2.10 This issue/PR affects Ansible v2.10 community_review In order to be merged, this PR must follow the community review workflow. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. support:community This issue/PR relates to code supported by the Ansible community. system System category labels Sep 19, 2019
@ansibot
Copy link
Contributor

ansibot commented Sep 19, 2019

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

lib/ansible/modules/system/xfconf.py:0:0: doc-choices-do-not-match-spec: Argument 'state' in argument_spec defines choices as (['present', 'get', 'absent']) but documentation defines choices as ([])
lib/ansible/modules/system/xfconf.py:0:0: doc-default-does-not-match-spec: Argument 'state' in argument_spec defines default as ('present') but documentation defines default as (None)
lib/ansible/modules/system/xfconf.py:25:10: documentation-syntax-error: DOCUMENTATION is not valid YAML

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

lib/ansible/modules/system/xfconf.py:184:0: trailing-whitespace: Trailing whitespace

The test ansible-test sanity --test ansible-doc [explain] failed with the error:

Command "ansible-doc -t module xfconf" returned exit status 1.
>>> Standard Error
ERROR! module xfconf missing documentation (or could not parse documentation): mapping values are not allowed in this context
  in "<unicode string>", line 10, column 10

The test ansible-test sanity --test docs-build [explain] failed with the error:

Command "/usr/bin/python3.6 /root/ansible/test/sanity/code-smell/docs-build.py" returned exit status 1.
>>> Standard Error
Command 'make singlehtmldocs' failed with status code: 2
--> Standard Output
PYTHONPATH=../../lib ../../hacking/build-ansible.py collection-meta --template-file=../templates/collections_galaxy_meta.rst.j2 --output-dir=rst/dev_guide/ ../../lib/ansible/galaxy/data/collections_galaxy_meta.yml
PYTHONPATH=../../lib ../../hacking/build-ansible.py document-config --template-file=../templates/config.rst.j2 --output-dir=rst/reference_appendices/ ../../lib/ansible/config/base.yml
mkdir -p rst/cli
PYTHONPATH=../../lib ../../hacking/build-ansible.py generate-man --template-file=../templates/cli_rst.j2 --output-dir=rst/cli/ --output-format rst ../../lib/ansible/cli/*.py
PYTHONPATH=../../lib ../../hacking/build-ansible.py document-keywords --template-dir=../templates --output-dir=rst/reference_appendices/ ./keyword_desc.yml
PYTHONPATH=../../lib ../../hacking/build-ansible.py document-plugins -t rst --template-dir=../templates --module-dir=../../lib/ansible/modules -o rst/modules/ 
Evaluating module files...
Makefile:99: recipe for target 'modules' failed
--> Standard Error
Traceback (most recent call last):
  File "../../hacking/build-ansible.py", line 92, in <module>
    main()
  File "../../hacking/build-ansible.py", line 81, in main
    retval = command.main(args)
  File "/root/ansible/hacking/build_library/build_ansible/command_plugins/plugin_formatter.py", line 721, in main
    plugin_info, categories = get_plugin_info(args.module_dir, limit_to=args.limit_to, verbose=(args.verbosity > 0))
  File "/root/ansible/hacking/build_library/build_ansible/command_plugins/plugin_formatter.py", line 225, in get_plugin_info
    doc, examples, returndocs, metadata = plugin_docs.get_docstring(module_path, fragment_loader, verbose=verbose)
  File "/root/ansible/lib/ansible/utils/plugin_docs.py", line 124, in get_docstring
    data = read_docstring(filename, verbose=verbose, ignore_errors=ignore_errors)
  File "/root/ansible/lib/ansible/parsing/plugin_docs.py", line 59, in read_docstring
    data[varkey] = AnsibleLoader(child.value.s, file_name=filename).get_single_data()
  File "/usr/local/lib/python3.6/dist-packages/yaml/constructor.py", line 41, in get_single_data
    node = self.get_single_node()
  File "ext/_yaml.pyx", line 707, in _yaml.CParser.get_single_node
  File "ext/_yaml.pyx", line 725, in _yaml.CParser._compose_document
  File "ext/_yaml.pyx", line 776, in _yaml.CParser._compose_node
  File "ext/_yaml.pyx", line 890, in _yaml.CParser._compose_mapping_node
  File "ext/_yaml.pyx", line 774, in _yaml.CParser._compose_node
  File "ext/_yaml.pyx", line 853, in _yaml.CParser._compose_sequence_node
  File "ext/_yaml.pyx", line 905, in _yaml.CParser._parse_next_event
yaml.scanner.ScannerError: mapping values are not allowed in this context
  in "<unicode string>", line 10, column 10
make: *** [modules] Error 1

The test ansible-test sanity --test package-data [explain] failed with the error:

Command "/usr/bin/python3.6 /root/ansible/test/sanity/code-smell/package-data.py" returned exit status 1.
>>> Standard Error
Traceback (most recent call last):
  File "/root/ansible/test/sanity/code-smell/package-data.py", line 383, in <module>
    main()
  File "/root/ansible/test/sanity/code-smell/package-data.py", line 360, in main
    sdist_path = create_sdist(tmp_dir)
  File "/root/ansible/test/sanity/code-smell/package-data.py", line 174, in create_sdist
    raise Exception('make snapshot failed:\n%s' % stderr)
Exception: make snapshot failed:
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
docs/man/man1/ansible-galaxy.1.rst:51: (WARNING/2) Definition list ends without a blank line; unexpected unindent.
docs/man/man1/ansible-galaxy.1.rst:57: (WARNING/2) Definition list ends without a blank line; unexpected unindent.
ERROR! module xfconf at /tmp/tmpwos5b91r/lib/ansible/modules/system/xfconf.py has a documentation error formatting or is missing documentation.
Traceback (most recent call last):
  File "packaging/release/changelogs/changelog.py", line 835, in <module>
    main()
  File "packaging/release/changelogs/changelog.py", line 102, in main
    args.func(args)
  File "packaging/release/changelogs/changelog.py", line 132, in command_release
    plugins = load_plugins(version=version, force_reload=reload_plugins)
  File "packaging/release/changelogs/changelog.py", line 184, in load_plugins
    '--json', '--metadata-dump', '-t', plugin_type])
  File "/usr/lib/python3.6/subprocess.py", line 356, in check_output
    **kwargs).stdout
  File "/usr/lib/python3.6/subprocess.py", line 438, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['/tmp/tmpwos5b91r/bin/ansible-doc', '--json', '--metadata-dump', '-t', 'module']' returned non-zero exit status 1.
make: *** [changelog] Error 1

The test ansible-test sanity --test pep8 [explain] failed with 3 errors:

lib/ansible/modules/system/xfconf.py:24:1: W293: blank line contains whitespace
lib/ansible/modules/system/xfconf.py:136:17: E126: continuation line over-indented for hanging indent
lib/ansible/modules/system/xfconf.py:184:1: W293: blank line contains whitespace

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

lib/ansible/modules/system/xfconf.py:25:10: error: DOCUMENTATION: syntax error: mapping values are not allowed here

click here for bot help

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Sep 19, 2019
@russoz
Copy link
Contributor Author

russoz commented Sep 19, 2019

and voilà.

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Sep 19, 2019
@russoz
Copy link
Contributor Author

russoz commented Sep 24, 2019

@ansibot ready_for_review

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 2, 2019
@jbenden
Copy link
Contributor

jbenden commented Oct 6, 2019

shipit

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Oct 6, 2019
- Revamped calculation of previous value
- Refactored some checks from main() to the XfConfProperty class
@ansibot
Copy link
Contributor

ansibot commented Oct 22, 2019

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

lib/ansible/modules/system/xfconf.py:0:0: doc-type-does-not-match-spec: Argument 'recursive' in argument_spec defines type as <class 'bool'> but documentation defines type as 'bool'

The test ansible-test sanity --test pylint [explain] failed with 3 errors:

lib/ansible/modules/system/xfconf.py:170:34: blacklisted-name: Black listed name "_"
lib/ansible/modules/system/xfconf.py:181:23: ansible-format-automatic-specification: Format string contains automatic field numbering specification
lib/ansible/modules/system/xfconf.py:224:35: ansible-format-automatic-specification: Format string contains automatic field numbering specification

click here for bot help

@ansibot ansibot removed community_review In order to be merged, this PR must follow the community review workflow. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Oct 22, 2019
@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. has_issue needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Oct 22, 2019
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Oct 22, 2019
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 30, 2019
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Mar 30, 2020
@ansibot ansibot added collection Related to Ansible Collections work collection:community.general needs_collection_redirect https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md labels Apr 29, 2020
@call-a3
Copy link

call-a3 commented Jun 5, 2020

Any update on this? I'd like to make use of this functionality ;) @russoz plans on resolving the conflicts? Should I take a stab myself?

@russoz
Copy link
Contributor Author

russoz commented Jun 7, 2020

Hi @call-a3 I just saw this update now. Will get on to it tomorrow (it's GMT+12 here).

Any idea if 2.10 is coming out soon? I am relatively new to contributing to the project and I saw no updates in the roadmap since October when I coded this.

@russoz
Copy link
Contributor Author

russoz commented Jun 9, 2020

Hi @call-a3 ,

It seems the situation has changed. As described in https://github.com/ansible-collections/overview/blob/master/README.rst the ansible repository is going through a major overhaul for 2.10, and part of that consisted in refactoring most of the modules out of the main repo.

So now this module has been migrated to the location:
https://github.com/ansible-collections/community.general/blob/master/plugins/modules/system/xfconf.py

It seems the existing PRs here at the ansible repo (like this one), for the migrated modules, will be cancelled in the future, so I have just created a new PR at:
ansible-collections/community.general#482

Let's see how it plays out over there.

In the mean time, if you don't want to wait until it comes out, you can copy it (it's just a single file) to your playbook, rename it from xfconf to xfconf_new or something like that and go ahead and use it.

Cheers

@mkrizek
Copy link
Contributor

mkrizek commented Jun 9, 2020

Closing per above. Migrated to ansible-collections/community.general#482.

If you have further questions please stop by IRC or the mailing list:

@mkrizek mkrizek closed this Jun 9, 2020
@ansible ansible locked and limited conversation to collaborators Jul 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.10 This issue/PR affects Ansible v2.10 collection:community.general collection Related to Ansible Collections work feature This issue/PR relates to a feature request. has_issue module This issue/PR relates to a module. needs_collection_redirect https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. new_contributor This PR is the first contribution by a new community member. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community. system System category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add arrays support for xfconf module
5 participants