Support to skip setting defaults for optional fields without default …

[grastogi23]

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

module_utils

ANSIBLE VERSION

ansible 2.1.1.0
  config file = 
  configured module search path = Default w/o overrides

SUMMARY

skip_post_default_settings flag allows caller to prevent setting of
null to all the optional fields that do not have default. The intention
of the module there is to not pass null values for items like int,
string, etc. This is particularly useful when the API do not expect None
values for optional fields. It is also more efficient.

…in argument spec

[abadger]

Sample module that could make use of this: https://github.com/avinetworks/ansible-modules-extras/blob/devel/network/avi/avi_pool.py

[abadger]

Looking at this and the use case in that module I'm inclined to say we aren't going to take this change. the module is trying to merge two schemas for two different APIs into a single schema. Although the needs of the two schemas are similar, they aren't the same and it's better code if they are separated. If the two schemas are separated then the need for something like this goes away.

The way I'd code this is more along these lines:

AVI_OBJ_SCHEMA = {
    "tenant": {"nullable": False},
    "ab_priority": {"min_value": 0, "max_value": 255},
}

def create_avi_obj_description(datasource):
    avi_obj_description = {}
    for avi_param, avi_param_desc in AVI_OBJ_SCHEMA.items():
        if not avi_param_desc.get('nullable', False) and datasource[avi_param] is None:
            continue
        if avi_param_desc.get('min_value', None) is not None and avi_param_desc['min_value'] < datasource[avi_param]:
            continue
       [...]
        avi_obj_description[avi_param] = data[avi_param]
    return avi_obj_description

def main():
    [...]
    avi_obj_description = create_avi_obj_description(module.params)
    [..]
    changed = not avi_obj_cmp(avi_obj_description, existing_obj)
   [...]
   rsp = api.post('pool', data=avi_obj_description,
                           tenant=tenant, tenant_uuid=tenant_uuid)

Leaving open for now in case other core team members would like to disagree about whether we should take this or not.

[bcoca]

Discussed at core meeting, I'm also opposed to this change, it adds unneeded complication to core code.

[grastogi23]

Thanks for discussing it. I do have code similar to one suggested by @abadger as part of my module. It allows me to keep the same schema exposed in playbooks and tasks consistent with the AVI's REST API objects.

As a publisher of API I would like to keep the same object definitions across different frameworks. Anyone trying to achieve that would hit the issue without this patch. I respect your decision to not add complexity and keep current convention of the Module interface.