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

[Discussion] Standardise change checking logic #95

Open
SDBrett opened this issue Aug 9, 2018 · 1 comment
Open

[Discussion] Standardise change checking logic #95

SDBrett opened this issue Aug 9, 2018 · 1 comment

Comments

@SDBrett
Copy link
Contributor

SDBrett commented Aug 9, 2018

Maintenance and creation of modules would be simpler if the logic used for detecting if a change has occurred was more consistent.
The goal of introducing a standard approach to this is reduce time spent troubleshooting logic issues, reduce false change detection and ease moving between modules maintained by others.

The current approach is to assume that there is a change by using props_match = False and proving otherwise.

With simple managed objects without child items, this works fine, but come quite complex which child items are included.

When a playbook is run, it takes the input provided and compares to the live system to determine if a change is made. This is the opposite to the above mentioned approach. This is assuming nothing has changed and the checks prove otherwise; or in code props_match = True.

This approach makes no real different to managed objects without child items, but makes a large difference with child times, especially multiple child items.

In addition to the change to assuming true and proving false is that a check system only needs to find a single difference, all individual differences are not required to found.

def check_changed():
    props_match = True
    if parent_mo:
        if module.params['state'] == 'absent':
            props_match = False
        else:
            props_match = parent_mo.check_prop_match(**kwargs)
        if props_match:
            for i in child_mo_settings:
                mo_1 = get_child_mo(i)
                props_match = mo_1.check_prop_match(**kwargs)
                if not props_match:
                    break
        if props_match:
            for i in child2_mo_settings:
                mo_2 = get_child_mo(i)
                props_match = mo_2.check_prop_match(**kwargs)
                if not props_match:
                    break
    else:
        props_match = False

    return props_match

The approach above has a compromise between if statement nesting and minimising the number of return statements.

Thoughts, opinions?

@dsoper2
Copy link
Contributor

dsoper2 commented Aug 14, 2018

Looks good - thanks for suggesting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants