[WIP] osx_defaults: Add dict support #3420

aiotter · 2021-09-22T06:53:11Z

SUMMARY

Add dict support to osx_default.
Fixes #238.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

osx_defaults.py

ADDITIONAL INFORMATION

Example usage:

- osx_defaults:
    domain: com.apple.finder
    key: FXInfoPanesExpanded
    type: dict
    value: {General: true, OpenWith: true}
    state: present

- osx_defaults:
    domain: com.apple.finder
    key: FXInfoPanesExpanded
    dict_add: MetaData
    type: bool
    value: yes
    state: present

CAUTION: This PR contains a bug. To make it fixed, I need some help. For more detail: #238 (comment)

ansibullbot · 2021-09-22T07:04:49Z

The test botmeta failed with 1 error:

.github/BOTMETA.yml:0:0: Author aiotter not mentioned as active or inactive maintainer for plugins/modules/system/osx_defaults.py (mentioned are: Akasurde, kyleabenson, martinm82, danieljaouen, indrajitr, notok)

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

plugins/modules/system/osx_defaults.py:0:0: doc-type-does-not-match-spec: Argument 'dict_add' in argument_spec defines type as 'str' but documentation defines type as 'string'
plugins/modules/system/osx_defaults.py:0:0: invalid-documentation: DOCUMENTATION.options.dict_add.type: not a valid value for dictionary value @ data['options']['dict_add']['type']. Got 'string'

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

plugins/modules/system/osx_defaults.py:0:0: doc-type-does-not-match-spec: Argument 'dict_add' in argument_spec defines type as 'str' but documentation defines type as 'string'
plugins/modules/system/osx_defaults.py:0:0: invalid-documentation: DOCUMENTATION.options.dict_add.type: not a valid value for dictionary value @ data['options']['dict_add']['type']. Got 'string'

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

plugins/modules/system/osx_defaults.py:0:0: doc-type-does-not-match-spec: Argument 'dict_add' in argument_spec defines type as 'str' but documentation defines type as 'string'
plugins/modules/system/osx_defaults.py:0:0: invalid-documentation: DOCUMENTATION.options.dict_add.type: not a valid value for dictionary value @ data['options']['dict_add']['type']. Got 'string'

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

plugins/modules/system/osx_defaults.py:0:0: doc-type-does-not-match-spec: Argument 'dict_add' in argument_spec defines type as 'str' but documentation defines type as 'string'
plugins/modules/system/osx_defaults.py:0:0: invalid-documentation: DOCUMENTATION.options.dict_add.type: not a valid value for dictionary value @ data['options']['dict_add']['type']. Got 'string'

click here for bot help

aiotter · 2021-09-22T07:30:15Z

It seems parsing the result of defaults export by plistlib can work. I'll take a look into this solution when I have some time.

In [13]: import subprocess, plistlib
    ...: plistlib.loads(subprocess.run(['defaults', 'export', 'com.apple.finder', '-'], capture_output=True).stdout)
Out[13]:
{'AppleShowAllFiles': True,
 'ComputerViewSettings': {'CustomViewStyleVersion': 1,
  'WindowState': {'ContainerShowSidebar': True, ...

Apparently I forgot adding myself as a maintainer. I'll do it later. And also fix type error on document.

decadent · 2021-09-22T08:16:37Z

Note that plistlib is different in Python 3 and 2, and Macos has 2 by default—which Ansible has to use as a consequence. While you seem to be using version 3.

It might be possible to create a parser/encoder for the defaults' dict format, considering that it's not too complex—but of course that's more involved than just calling a library.

aiotter · 2021-09-22T11:48:49Z

Thank you @decadent!

It seems we can use plistlib.readPlistFromString on python2.7. Tested and worked.

>>> import subprocess, plistlib
>>> plistlib.readPlistFromString(subprocess.check_output(['defaults', 'export', 'com.apple.finder', '-']))
{'RecentMoveAndCopyDestinations': ['file:///Users/aiotter/Documents/', ...

But in this case we still have to take it into consideration of the case we are running ansible on python3.

I'm not familiar with the python2 compatibility. Is there some standard way in ansible to switch the code based on version of the python, or can I just do it with sys.version_info?

This commit contains a bug that all the dict manipulation flags the result `unchanged`. Need to be fixed.

aiotter · 2021-09-22T12:42:25Z

Used six.PY2 to switch the code.
Now I'm looking into failed CI.

felixfontein · 2021-09-22T20:41:09Z

plugins/modules/system/osx_defaults.py

+        if PY2:
+            self.current_value = plistlib.readPlistFromString(out)
+        else:
+            self.current_value = plistlib.loads(out.encode('utf-8'))


It might be a good idea to move this into a static function (that can easily be tested), and add unit tests for this. These are run with all Python versions Ansible supports, which will give you some confidence that this does what it is supposed to on all Python versions. Unit tests for this module would have to do into tests/unit/plugins/modules/system/test_osx_defaults.py (which doesn't exist yet).

I'm not very experienced with unit tests in Python. I read some of tests/unit/plugins/modules/system/* but it seems difficult to understand. Is there good document for this?

move this into a static function

Like this?

def convert_plist_xml_to_object(xml): if six.PY2: return plistlib.readPlistFromString(xml) else: return plistlib.loads(xml.encode('utf-8'))

You can find some information here: https://docs.ansible.com/ansible/latest/dev_guide/testing_units.html#structuring-unit-tests

And yes, such a function would help.

ansibullbot · 2021-09-28T20:48:48Z

cc @!UNKNOWN @Akasurde @danieljaouen @indrajitr @kyleabenson @martinm82 @notok
click here for bot help

martinm82 · 2021-10-19T07:21:31Z

plugins/modules/system/osx_defaults.py

    default: string
  array_add:
    description:
      - Add new elements to the array for a key which has an array as its value.
    type: bool
    default: no
+  dict_add:


Worth to add as well an example in EXAMPLES below.

russoz · 2022-03-12T04:32:21Z

Hi @aiotter how do you want to proceed with this PR?

russoz · 2022-04-15T10:47:02Z

needs_info

ansibullbot · 2022-05-21T06:09:13Z

@aiotter This pullrequest is waiting for your response. Please respond or the pullrequest will be closed.

click here for bot help

aiotter · 2022-05-23T04:12:48Z

Sorry that I have no time to carry on this PR.
If there are someone that want to implement this, you can add some tests so that it will be merged.

felixfontein · 2022-05-23T05:15:55Z

@aiotter thanks for your work on this PR! I hope that someone will pick it up and complete it!

ansibullbot added WIP Work in progress feature This issue/PR relates to a feature request module module new_contributor Help guide this first time contributor plugins plugin (any type) system labels Sep 22, 2021

Allow osx_defaults accept type dict

4cb8abe

This commit contains a bug that all the dict manipulation flags the result `unchanged`. Need to be fixed.

aiotter force-pushed the dev/osx_defaults branch from 075e2d0 to 4cb8abe Compare September 22, 2021 11:59

aiotter added 2 commits September 22, 2021 21:01

Add aiotter to the maintainers of osx_defaults

3b59175

osx_defaults: Use plistlib to get current value

6441c63

osx_defaults: Allow array input

8bfda0a

felixfontein added backport-3 check-before-release PR will be looked at again shortly before release and merged if possible. labels Sep 22, 2021

felixfontein reviewed Sep 22, 2021

View reviewed changes

ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Oct 6, 2021

felixfontein removed the backport-3 label Oct 12, 2021

martinm82 suggested changes Oct 19, 2021

View reviewed changes

felixfontein added the backport-4 label Nov 6, 2021

ansibullbot added the needs_info This issue requires further information. Please answer any outstanding questions label Apr 15, 2022

felixfontein removed the backport-4 label Apr 27, 2022

aiotter closed this May 23, 2022

aiotter mentioned this pull request Jul 20, 2022

osx_defaults - Add dict support #238

Open

andrea11 added a commit to andrea11/setup that referenced this pull request Jul 25, 2023

fix defaults (http://ansible-collections/community.general#3420)

147c513

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] osx_defaults: Add dict support #3420

[WIP] osx_defaults: Add dict support #3420

aiotter commented Sep 22, 2021

ansibullbot commented Sep 22, 2021

aiotter commented Sep 22, 2021

decadent commented Sep 22, 2021

aiotter commented Sep 22, 2021 •

edited

aiotter commented Sep 22, 2021

felixfontein Sep 22, 2021

aiotter Sep 23, 2021

aiotter Sep 23, 2021

felixfontein Sep 24, 2021

ansibullbot commented Sep 28, 2021

martinm82 Oct 19, 2021

russoz commented Mar 12, 2022

russoz commented Apr 15, 2022

ansibullbot commented May 21, 2022

aiotter commented May 23, 2022

felixfontein commented May 23, 2022

[WIP] osx_defaults: Add dict support #3420

[WIP] osx_defaults: Add dict support #3420

Conversation

aiotter commented Sep 22, 2021

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

ansibullbot commented Sep 22, 2021

aiotter commented Sep 22, 2021

decadent commented Sep 22, 2021

aiotter commented Sep 22, 2021 • edited

aiotter commented Sep 22, 2021

felixfontein Sep 22, 2021

Choose a reason for hiding this comment

aiotter Sep 23, 2021

Choose a reason for hiding this comment

aiotter Sep 23, 2021

Choose a reason for hiding this comment

felixfontein Sep 24, 2021

Choose a reason for hiding this comment

ansibullbot commented Sep 28, 2021

martinm82 Oct 19, 2021

Choose a reason for hiding this comment

russoz commented Mar 12, 2022

russoz commented Apr 15, 2022

ansibullbot commented May 21, 2022

aiotter commented May 23, 2022

felixfontein commented May 23, 2022

aiotter commented Sep 22, 2021 •

edited