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

combine filter: fine list handling (option b) #57894

Merged
merged 9 commits into from
Feb 12, 2020

Conversation

tchernomax
Copy link
Contributor

@tchernomax tchernomax commented Jun 15, 2019

SUMMARY

see:

args:

  • recursive: same as before
  • list_merge:
    • keep: same as before
    • append, prepend: list are appended
    • append_np, prepend_np: only elements that isn't already present in left list are append (_np stands for "non present")

README: will come if this implementation is validated.

I didn't implement 'uniq' as I think 'append_np' and 'prepend_np' are more useful, and are equivalent to unique if list are already filtered from duplication.

non backward compatible: [dict1, [dict2]] | combine doesn't work anymore, because I thought it shouldn't. If you disagree, flatten(terms, levels=1) → flatten(terms) and it's fixed.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

component =lib/ansible/plugins/filter/core.py

ADDITIONAL INFORMATION
max@mde-oxalide % cat toto.yml
- hosts: localhost
  gather_facts: false
  vars:
    toto_default:
      a:
        a:
          a: default_value
          b: default_value
      b:
        - 1
        - 1
        - 2
    toto:
      a:
        a:
          b: toto_value
          c: toto_value
      b:
        - 2
        - 3
        - 3
        - 4: toto_value
  tasks:
    - name: recursive=False, list_merge='keep'
      debug:
        msg: "{{toto_default|toto_combine(toto, recursive=False, list_merge='keep')}}"
    - name: recursive=False, list_merge='append'
      debug:
        msg: "{{toto_default|toto_combine(toto, recursive=False, list_merge='append')}}"
    - name: recursive=False, list_merge='prepend'
      debug:
        msg: "{{toto_default|toto_combine(toto, recursive=False, list_merge='prepend')}}"
    - name: recursive=False, list_merge='append_np'
      debug:
        msg: "{{toto_default|toto_combine(toto, recursive=False, list_merge='append_np')}}"
    - name: recursive=False, list_merge='prepend_np'
      debug:
        msg: "{{toto_default|toto_combine(toto, recursive=False, list_merge='prepend_np')}}"

    - name: recursive=True, list_merge='keep'
      debug:
        msg: "{{toto_default|toto_combine(toto, recursive=True, list_merge='keep')}}"
    - name: recursive=True, list_merge='append'
      debug:
        msg: "{{toto_default|toto_combine(toto, recursive=True, list_merge='append')}}"
    - name: recursive=True, list_merge='prepend'
      debug:
        msg: "{{toto_default|toto_combine(toto, recursive=True, list_merge='prepend')}}"
    - name: recursive=True, list_merge='append_np'
      debug:
        msg: "{{toto_default|toto_combine(toto, recursive=True, list_merge='append_np')}}"
    - name: recursive=True, list_merge='prepend_np'
      debug:
        msg: "{{toto_default|toto_combine(toto, recursive=True, list_merge='prepend_np')}}"
PLAY [localhost] ***********************************************************************

TASK [recursive=False, list_merge='keep'] **********************************************
ok: [localhost] => {
    "msg": {
        "a": {
            "a": {
                "b": "toto_value",
                "c": "toto_value"
            }
        },
        "b": [
            2,
            3,
            3,
            {
                "4": "toto_value"
            }
        ]
    }
}

TASK [recursive=False, list_merge='append'] ********************************************
ok: [localhost] => {
    "msg": {
        "a": {
            "a": {
                "b": "toto_value",
                "c": "toto_value"
            }
        },
        "b": [
            1,
            1,
            2,
            2,
            3,
            3,
            {
                "4": "toto_value"
            }
        ]
    }
}

TASK [recursive=False, list_merge='prepend'] *******************************************
ok: [localhost] => {
    "msg": {
        "a": {
            "a": {
                "b": "toto_value",
                "c": "toto_value"
            }
        },
        "b": [
            1,
            1,
            2,
            2,
            3,
            3,
            {
                "4": "toto_value"
            }
        ]
    }
}

TASK [recursive=False, list_merge='append_np'] *****************************************
ok: [localhost] => {
    "msg": {
        "a": {
            "a": {
                "b": "toto_value",
                "c": "toto_value"
            }
        },
        "b": [
            2,
            3,
            3,
            {
                "4": "toto_value"
            },
            1,
            1
        ]
    }
}

TASK [recursive=False, list_merge='prepend_np'] ****************************************
ok: [localhost] => {
    "msg": {
        "a": {
            "a": {
                "b": "toto_value",
                "c": "toto_value"
            }
        },
        "b": [
            1,
            1,
            2,
            3,
            3,
            {
                "4": "toto_value"
            }
        ]
    }
}

TASK [recursive=True, list_merge='keep'] ***********************************************
ok: [localhost] => {
    "msg": {
        "a": {
            "a": {
                "a": "default_value",
                "b": "toto_value",
                "c": "toto_value"
            }
        },
        "b": [
            2,
            3,
            3,
            {
                "4": "toto_value"
            }
        ]
    }
}

TASK [recursive=True, list_merge='append'] *********************************************
ok: [localhost] => {
    "msg": {
        "a": {
            "a": {
                "a": "default_value",
                "b": "toto_value",
                "c": "toto_value"
            }
        },
        "b": [
            1,
            1,
            2,
            2,
            3,
            3,
            {
                "4": "toto_value"
            }
        ]
    }
}

TASK [recursive=True, list_merge='prepend'] ********************************************
ok: [localhost] => {
    "msg": {
        "a": {
            "a": {
                "a": "default_value",
                "b": "toto_value",
                "c": "toto_value"
            }
        },
        "b": [
            1,
            1,
            2,
            2,
            3,
            3,
            {
                "4": "toto_value"
            }
        ]
    }
}

TASK [recursive=True, list_merge='append_np'] ******************************************
ok: [localhost] => {
    "msg": {
        "a": {
            "a": {
                "a": "default_value",
                "b": "toto_value",
                "c": "toto_value"
            }
        },
        "b": [
            2,
            3,
            3,
            {
                "4": "toto_value"
            },
            1,
            1
        ]
    }
}

TASK [recursive=True, list_merge='prepend_np'] *****************************************
ok: [localhost] => {
    "msg": {
        "a": {
            "a": {
                "a": "default_value",
                "b": "toto_value",
                "c": "toto_value"
            }
        },
        "b": [
            1,
            1,
            2,
            3,
            3,
            {
                "4": "toto_value"
            }
        ]
    }
}

PLAY RECAP *****************************************************************************
localhost                  : ok=10   changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 core_review In order to be merged, this PR must follow the core review workflow. feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jun 15, 2019
@ansibot
Copy link
Contributor

ansibot commented Jun 15, 2019

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/plugins/filter/core.py:314:8: undefined-variable Undefined variable '_validate_mutable_mappings'

The test ansible-test sanity --test pep8 [explain] failed with 3 errors:

lib/ansible/plugins/filter/core.py:320:17: E117 over-indented
lib/ansible/plugins/filter/core.py:362:22: E261 at least two spaces before inline comment
lib/ansible/plugins/filter/core.py:374:1: E302 expected 2 blank lines, found 1

click here for bot help

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Jun 15, 2019
@tchernomax tchernomax force-pushed the combine_merge_array_b branch 4 times, most recently from efd032e to af365bb Compare June 16, 2019 12:55
@Akasurde Akasurde requested review from sivel and bcoca June 16, 2019 15:28
@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Jun 18, 2019
lib/ansible/plugins/filter/core.py Outdated Show resolved Hide resolved
@tchernomax tchernomax force-pushed the combine_merge_array_b branch 4 times, most recently from da849c9 to c3f0bbe Compare June 18, 2019 23:05
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jun 27, 2019
@ansibot ansibot added the stale_review Updates were made after the last review and the last review is more than 7 days old. label Jul 5, 2019
@tchernomax tchernomax changed the title combine filter: fine list handling WIP combine filter: fine list handling (option b) Jul 7, 2019
@ansibot ansibot added the WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. label Jul 7, 2019
@tchernomax
Copy link
Contributor Author

tchernomax commented Jul 7, 2019

ping @bcoca (I made the change you asked for)

@ansibot ansibot added the stale_review Updates were made after the last review and the last review is more than 7 days old. label Dec 27, 2019
@ansibot ansibot 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. and removed core_review In order to be merged, this PR must follow the core review workflow. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Feb 5, 2020
@samccann
Copy link
Contributor

@tchernomax can you please rebase so I can review the docs portion of this PR? thanks!

Maxime de Roucy and others added 9 commits February 11, 2020 21:31
@ansibot ansibot removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Feb 11, 2020
@tchernomax
Copy link
Contributor Author

tchernomax commented Feb 11, 2020

Can someone relaunch the CI, I think the fail has nothing to do with my commit.

ping @samccann @felixfontein

@felixfontein
Copy link
Contributor

CI restarted.

@tchernomax
Copy link
Contributor Author

thanks felix

@samccann can you review ?

(it would be awesome to see this one year old PR moving forward !)

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Feb 11, 2020
Copy link
Contributor

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@samccann samccann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs LTGM

@samccann samccann merged commit 53e043b into ansible:devel Feb 12, 2020
@samccann
Copy link
Contributor

Thanks @tchernomax for your patience on this Ansible fix!

@ansible ansible locked and limited conversation to collaborators Mar 11, 2020
@tchernomax tchernomax deleted the combine_merge_array_b branch March 24, 2020 13:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 core_review In order to be merged, this PR must follow the core review workflow. docs This issue/PR relates to or includes documentation. feature This issue/PR relates to a feature request. has_issue support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deep merge of dictionaries Appending or "stacking" variable lists
9 participants