Why do modules prefer dict constructor over dict literal?

Get Help
,
1

Hi, :wave:

I’ve been looking through some modules in ansible/lib/ansible/modules (but also community.general/plugins/modules) and all modules I’ve looked at so far do prefer the dict constructor when constructing the argument_spec for the AnsibleModule constructor.

Here’s an example from blockinfile.py

module = AnsibleModule(
        argument_spec=dict(
            path=dict(type='path', required=True, aliases=['dest', 'destfile', 'name']),
            state=dict(type='str', default='present', choices=['absent', 'present']),
            marker=dict(type='str', default='# {mark} ANSIBLE MANAGED BLOCK'),
            ...
        ),
        ...

The latest documentation on developing modules also suggests this syntax.

I’ve been wondering why the dict constructor is preferred over dict literal. pylint for example has a specfic rule R1735 to discourage this (more details on the reasons can be found here: Prefer dict literal over `dict` constructor · Issue #7690 · pylint-dev/pylint · GitHub).

Does anyone have any insight or pointers?

2 Likes
2

That’s an excellent question!

I have no idea where this comes from (dict literals have been supported since at least Python 2.0: 5.2.5 Dictionary displays, so it’s not because of “Python 2.1 did not support that yet” or a similar argument - not sure whether ansible ever supported Python 1 or whether it was available there though).

One advantage of using dict() over {} is that option names are forced to be Python identifiers. And that you have to type less quotes and curly brackets. Depending on your keyboard layout, that can be useful (thinking of German keyboard layouts where curly brackets are harder to type than round brackets).

Besides that, it’s mostly “we always did it that way”, I guess. I got so used to the dict() syntax that I also use it for new modules. (I wasn’t aware it’s slower than dict literals, though the performance difference shouldn’t be relevant when using it in argument specs as the time needed to run that code is absolutely tiny compared to the effort of executing the module itself, which itself is dwarfed by the time needed to copy the module to the remote machine.)

I guess it would be a good idea to update the docs. That would at least ensure that new module authors hopefully use that as a template instead of continuing to use dict() :slight_smile:

3 Likes
3

Thanks a bunch for your explanation!

That’s what I thought :wink:

4

This is really interesting! I use dict() because when I started with ansible all modules that I’ve seen used this, too. So it’s really a case of “I always did it that way”.

@Core Out of curiosity, are there any reasons to prefer dict() over {} from your point of view?

@felixfontein Do you think this is worth discussing (or at least mentioning) in the next community meeting? If you do, let’s add the community-wg-nextmtg so we don’t forget.

2 Likes
5

For a module argument spec, the improved readability and ease of typing unquoted keys for dict() outweighs the negligible performance gain of {}.

3 Likes
6

I second @mattclay on the improved readability. I think it looks much cleaner in the editor than with all those boilerplate quotes around.

1 Like
7

This was a specific choice made bye MPD (Ansible’s original author), mostly for the reasons @mattclay states.

The fact that modules execute as a ‘script one off’ on the targets so some performance considerations like {} vs dict() have basically no gain for each task so readability and maintainability was always chosen over minor performance optimizations. These would make more sense in case of a long time running process/daemon and frequent reuse of a function, both of which do not apply to the argument spec.

3 Likes
8

And, in 2024 (remembering old classes about compilers some 25 years ago), I would expect the compiler to be smart enough to understand that a dict() initialized exclusively with literals (guesstimating >90% of the modules) can be optimized to generate the exact same bytecode as a {} literal. I mean, it should, right?

9

Just a regular user here. I don’t have a horse in this race. That being said I don’t particularly enjoy dancing this weird dance with pylint to disable R1735 just for the argument_spec (I do still appreciate this warning everywhere else) so I’ve switched to using the dict literal for my modules.

I don’t think this

module = AnsibleModule(
        argument_spec={
            'path': {'type': 'path', 'required': True, 'aliases': ['dest', 'destfile', 'name']},
            'state': {'type': 'str', 'default': 'present', 'choices': ['absent', 'present']},
            'marker': {'type': 'str', 'default': '# {mark} ANSIBLE MANAGED BLOCK'},
            ...
        ),
        ...

or even this

module = AnsibleModule(
        argument_spec={
            'path': {
                'type': 'path',
                'required': True,
                'aliases': ['dest', 'destfile', 'name']
            },
            'state': {
                'type': 'str',
                'default': 'present',
                'choices': ['absent', 'present']
            },
            'marker': {
                'type': 'str',
                'default': '# {mark} ANSIBLE MANAGED BLOCK'
            },
            ...
        ),
        ...

looks so much worse than this:

module = AnsibleModule(
        argument_spec=dict(
            path=dict(type='path', required=True, aliases=['dest', 'destfile', 'name']),
            state=dict(type='str', default='present', choices=['absent', 'present']),
            marker=dict(type='str', default='# {mark} ANSIBLE MANAGED BLOCK'),
            ...
        ),
        ...

Just food for thought.

1 Like
10

That’s JSON, so if is was written as YAML (would that be possible?) it’s:

path:
  type: path
  required: true
  aliases:
    - dest
    - destfile
    - name
state:
  type: str
  default: present
  choices:
    - absent
    - present
marker:
  type: str
  default: '# {mark} ANSIBLE MANAGED BLOCK'

That would be my choice for readability, compared with:

However I’m not an Python programmer… :person_shrugging:

11

No, it’s not. It’s Python code, and while the syntax in this particular section of code is similar in some ways to JSON they’re not at all interchangeable. Among other things, JSON would not allow you to use single quotes around the strings.

3 Likes
12

For parsing YAML you need a YAML parser (like pyyaml). While there’s definitely one around on the controller, it is usually not present on the targets. Thus being allowed to use YAML in modules would add the requirement of a YAML parser to all these modules.

(The nice thing about Ansible is that many modules only require Python on the target, nothing else. Having to install something should only be necessary if doing it without that would be really hard or complex. “I want to write my argument spec in a nicer way” doesn’t qualify IMO to force users to install another requirement everywhere before using a module.)

2 Likes
13

I disagree, the syntax highlighting makes this a lot easier to parse than the other versions you posted - there all keys are strings as well and thus formatted the same way as the string values. Here you see keys and values formatted differently.

2 Likes
14

This topic was automatically closed 30 days after the last reply. New replies are no longer allowed.