dconf: add feature to populate a subpath from file #10432

Sh3idan · 2025-07-19T08:23:01Z

SUMMARY

This merge request allows to populate a subpath from file by using load sub-command of the dconf binary. Below the changes that were made:

state: load has been added to address the feature
path is a new option working with the feature. It points to a file located in node which contains a subpath dump.
key remains unchanged but, here, it refers to a root directory (instead of a dconf key) and must be '/' terminated

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

dconf

ADDITIONAL INFORMATION

Below a role using the newly load state, followed by the varbatim output:

- name: Apply profile for Gnome terminal
  ansible.builtin.dconf:
    key: "/org/gnome/terminal/legacy/profiles:/"
    path: "/tmp/solarized_dark.dump"
    state: load

changed: [192.168.1.1] => {
    "changed": true,
    "invocation": {
        "module_args": {
            "key": "/org/gnome/terminal/legacy/profiles:/",
            "path": "/tmp/solarized_dark.dump",
            "state": "load",
            "value": null
        }
    }
}

The content of the path file is:

[:b1daa9dd-6252-4d8d-a863-c897e6d619b9]
background-color='rgb(0,43,54)'
bold-is-bright=false
foreground-color='rgb(131,148,150)'
palette=['rgb(7,54,66)', 'rgb(220,50,47)', 'rgb(133,153,0)', 'rgb(181,137,0)', 'rgb(38,139,210)', 'rgb(211,54,130)', 'rgb(42,161,152)', 'rgb(238,232,213)', 'rgb(0,43,54)', 'rgb(203,75,22)', 'rgb(88,110,117)', 'rgb(101,123,131)', 'rgb(131,148,150)', 'rgb(108,113,196)', 'rgb(147,161,161)', 'rgb(253,246,227)']
use-theme-colors=false

* Simplify check of changes

ansibullbot · 2025-07-19T08:23:23Z

cc @azaghal
click here for bot help

felixfontein

Thanks for your contribution!

plugins/modules/dconf.py

Co-authored-by: Felix Fontein <[email protected]>

* Mutual exclusion of 'path' and 'value'

plugins/modules/dconf.py

felixfontein · 2025-07-20T13:53:36Z

plugins/modules/dconf.py

+            # NOTE: This case shouldn't happen yet as 'key' and 'path' are
+            #       required with 'state=present'
+            # TODO: if 'key' ends with '/' then 'dconf list' should be used
+            #       else, 'dconf read'


Shouldn't this TODO be part of dconf.load()?

I don't thing so.

The architecture defined by the original author suggests that:

dconf.read handles dconf read [key]

dconf.write handles dconf write [key] [value]

dconf.load handles dconf load [key] < [path] (this contribution)

If key ends with '/', it means that it's a directory. If value is set, it should be illegal. If path is set, we're in the case of the contribution. If none of them is set, user tries to read a directory. It cannot be handled neither by dconf read nor dconf dump instead, dconf list must be used. In the future, dconf.list must be created to by compliant.

As you said, state=read is a mistake. As I understand, it means that reading key (whatever the meaning - a key or a directory) should have been handled by state=present. That's why I added this TODO here.

Are you agree with that?

As you said, state=read is a mistake. As I understand, it means that reading key (whatever the meaning - a key or a directory) should have been handled by state=present. That's why I added this TODO here.

What do you mean that read is a mistake? The read state is meant to read the key value without making changes to it - the present state will/might change the value. Or have I misunderstood something here?

Hi,

Felix wrote the following sentence:

Note that load is not a state. (Not that read is one, but that mistake has already been made in the past.)

I understand that state=read shouldn't be valid. If it's a valid state, that's fine with me!

state=read should not be there, it should be moved to a separate _info module. Since that state has been added a long time ago (together with the module in 2017), it's still there until someone invests some work to move it to a separate module though. But if someone tries to add a similar "state" again to a module nowadays, it would be rejected.

Anyway, this comment has been removed to follow the guidance of russoz!

felixfontein · 2025-07-20T13:54:19Z

plugins/modules/dconf.py

+
+        # Run the command and fetch standard return code, stdout, and stderr.
+        dbus_wrapper = DBusWrapper(self.module)
+        rc, out, err = dbus_wrapper.run_command(command, data=raw_config)


What happens to keys that already exist, but are not part of raw_config? Are these kept, or removed?

If the latter, the changed check above is not correct.

If keys exist in the database but, are not part of raw_config then they're kept. changed check should be correct!

Co-authored-by: Felix Fontein <[email protected]>

This update allows: * Compatibility between python2 and python3 * To handle IO operation errors Co-authored-by: Felix Fontein <[email protected]>

Co-authored-by: Felix Fontein <[email protected]>

Sh3idan · 2025-07-20T22:07:13Z

Note that I added a sub-function (_create_dconf_key) to fix a bug when raw_config contains entry at the root.

russoz

hi @Sh3idan, thanks for your contribution.

I have some other comments as well.

plugins/modules/dconf.py

russoz · 2025-07-20T07:16:18Z

plugins/modules/dconf.py

+        changed = any(
+            not self.variants_are_equal(self.read("%s%s/%s" % (root_dir, sub_dir, k)), v)
+            for sub_dir in config.sections()
+            for k, v in config[sub_dir].items()
+        )


So, I presume you are doing this because dconf will not report whether any value was changed, is that so?

Yes, that's why i'm doing this. Should I also check if the keys have actually been added?

Only if you have reason to believe that dconf might not add them and be silent about it. If we can trust them to be added (or receive an error if something goes bad), then there is no need to perform additional checks.

changelogs/fragments/10432-dconf-load-subpath.yml

Co-authored-by: Alexei Znamensky <[email protected]>

* Remove unreachable code

felixfontein · 2025-07-26T14:27:40Z

changelogs/fragments/10432-dconf-load-subpath.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - dconf - adds support of ``load`` to populate a subpath from file (https://github.com/ansible-collections/community.general/pull/10432).


Suggested change

- dconf - adds support of ``load`` to populate a subpath from file (https://github.com/ansible-collections/community.general/pull/10432).

- dconf - allow to populate a subpath from a file (https://github.com/ansible-collections/community.general/pull/10432).

felixfontein · 2025-07-26T14:29:33Z

plugins/modules/dconf.py

    elif module.params['state'] == 'present':
-        changed = dconf.write(module.params['key'], module.params['value'])
-        module.exit_json(changed=changed)
+        if module.params['path']:


Suggested change

if module.params['path']:

if module.params['path'] is not None:

felixfontein · 2025-07-26T14:29:40Z

plugins/modules/dconf.py

+            # Use 'dconf load' to propagate multiple entries from the root given by 'key'
+            changed = dconf.load(module.params['key'], module.params['path'])
+            module.exit_json(changed=changed)
+        elif module.params['value']:


Suggested change

elif module.params['value']:

else:

felixfontein · 2025-07-26T14:30:53Z

plugins/modules/dconf.py


    # Process based on different states.
    if module.params['state'] == 'read':
+        # TODO: Handle this case when 'state=present' and 'key' is the only one


I'm not sure what this comment means. Can you please explain it?

felixfontein · 2025-09-04T05:07:22Z

@Sh3idan ping

needs_info

Sh3idan added 3 commits July 15, 2025 18:23

feat: populate a subpath from stdin using load command

29b11f0

* Use 'path' convention

7443fe4

* Simplify check of changes

Fix typo

b840120

ansibullbot added feature This issue/PR relates to a feature request module module needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR new_contributor Help guide this first time contributor plugins plugin (any type) labels Jul 19, 2025

Add changelog fragment

63f22d3

This comment was marked as outdated.

Sign in to view

ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Jul 19, 2025

Fix pycodestyle

6f8ef18

ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jul 19, 2025

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-11 Automatically create a backport for the stable-10 branch labels Jul 19, 2025

felixfontein reviewed Jul 19, 2025

View reviewed changes

plugins/modules/dconf.py Outdated Show resolved Hide resolved

plugins/modules/dconf.py Outdated Show resolved Hide resolved

Sh3idan and others added 2 commits July 19, 2025 13:28

Fix path type in doc

266b62b

Co-authored-by: Felix Fontein <[email protected]>

Fix path and value are mutually exclusive

84df105

This comment was marked as outdated.

Sign in to view

ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jul 19, 2025

Sh3idan added 2 commits July 19, 2025 09:58

Fix trailing spaces

3f37370

* Use state=present instead of state=load

840fbf8

* Mutual exclusion of 'path' and 'value'

ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jul 19, 2025

felixfontein reviewed Jul 20, 2025

View reviewed changes

Sh3idan and others added 2 commits July 20, 2025 13:10

Fix issue when creating dconf key

a7dd03f

Update 'value' description

fffdfb4

Co-authored-by: Felix Fontein <[email protected]>

ansibullbot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jul 20, 2025

Sh3idan and others added 2 commits July 20, 2025 21:49

Update 'path' description

8bdcdc8

Co-authored-by: Felix Fontein <[email protected]>

Add trailing comma to avoid future diff

d860986

Co-authored-by: Felix Fontein <[email protected]>

ansibullbot removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jul 20, 2025

Sh3idan and others added 5 commits July 20, 2025 22:21

Update: handling IO operation errors

bc32bd4

This update allows: * Compatibility between python2 and python3 * To handle IO operation errors Co-authored-by: Felix Fontein <[email protected]>

Fix error message when reading config file failed

236edae

Co-authored-by: Felix Fontein <[email protected]>

Fix typo in a comment

7e3f675

Co-authored-by: Felix Fontein <[email protected]>

Fix configparser package to support py2 and py3

abba302

Co-authored-by: Felix Fontein <[email protected]>

Fix error message when parsing config failed

6e10135

Co-authored-by: Felix Fontein <[email protected]>

russoz reviewed Jul 21, 2025

View reviewed changes

Sh3idan and others added 3 commits July 22, 2025 21:47

Fix redundant args for 'value' and 'path'

8177c0a

Co-authored-by: Alexei Znamensky <[email protected]>

Fix fragment format

07a6c79

Co-authored-by: Alexei Znamensky <[email protected]>

* Remove redundant type in the 'path' documentation

588eb3c

* Remove unreachable code

felixfontein reviewed Jul 26, 2025

View reviewed changes

russoz changed the title ~~dconf_module: add feature: populate a subpath from file~~ dconf: add feature to populate a subpath from file Jul 27, 2025

ansibullbot 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 stale_ci CI is older than 7 days, rerun before merging labels Aug 4, 2025

ansibullbot added the needs_info This issue requires further information. Please answer any outstanding questions label Sep 4, 2025

		@@ -0,0 +1,2 @@
		minor_changes:
		- dconf - adds support of ``load`` to populate a subpath from file (https://github.com/ansible-collections/community.general/pull/10432).

	- dconf - adds support of ``load`` to populate a subpath from file (https://github.com/ansible-collections/community.general/pull/10432).
	- dconf - allow to populate a subpath from a file (https://github.com/ansible-collections/community.general/pull/10432).

	if module.params['path']:
	if module.params['path'] is not None:

dconf: add feature to populate a subpath from file #10432

Are you sure you want to change the base?

dconf: add feature to populate a subpath from file #10432

Uh oh!

Conversation

Sh3idan commented Jul 19, 2025

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

Uh oh!

ansibullbot commented Jul 19, 2025

Uh oh!

This comment was marked as outdated.

felixfontein left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sh3idan commented Jul 20, 2025

Uh oh!

russoz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sh3idan Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felixfontein commented Sep 4, 2025

Uh oh!

Uh oh!

Sh3idan Jul 22, 2025 •

edited

Loading