)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"fc4c44b7aaf1ba022730ead0a3dcf64b5aeda5b5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"7dee32ad_23718b86","updated":"2023-01-18 09:11:24.000000000","message":"module_dict_default \u003d {\"some_option\": \"some_default_value\"}\nuser_common_options_dict \u003d \"{\"some_option\" : \"some_user_defined_value\"}\"\n\nCurrent buggy code is doing this : \n\nResult :   {\"some_option\" : \"some_default_value\"}   \n\ninsted of \n\nResult : {\"some_option\" : \"some_user_defined_value\"}","commit_id":"70d68962364e47072df82d38f93319aa39cf99fa"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"3a1e685ad449595259807ff8d7252a63c29e4a17","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b26bbede_cd19d9ff","updated":"2023-01-18 14:56:53.000000000","message":"fixed pythonic way","commit_id":"97dc735d0da79044ea0f72a5670e06497cf28151"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"836bb9706fdde8b8231ac693014e0bfdabdeb52e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"06c5d5ba_b62a6fde","updated":"2023-01-31 02:15:26.000000000","message":"Folks, can u review again please ?  Tested locally ","commit_id":"bb179fdbf8a7db9e857c2666b65d54c982547b8c"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"8002e1ed348b81eaea02e5772ca82e2cc52a15cd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"bea3ee01_eb827184","updated":"2023-02-06 09:41:15.000000000","message":"Ready for review.","commit_id":"63b9fa56390796c93c76f3bb121015973663d9e7"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"e8ae7132c59c89ea165167099e5292a92400bafa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"703c7d96_8c24caab","updated":"2023-02-05 08:33:57.000000000","message":"checking http://169.254.169.254/2009-04-04/instance-id\nfailed 1/20: up 134.33. request failed\nfailed 2/20: up 137.51. request failed\nsuccessful after 3/20 tries: up 140.98. iid\u003di-00000001\n\nIt looks like it takes longer to start instance on the Rocky than on other distros.","commit_id":"63b9fa56390796c93c76f3bb121015973663d9e7"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"1459a65011e6fcecf23e156c79b659f92563e59c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"2b7113c7_e2723615","updated":"2023-02-04 23:18:27.000000000","message":"now It will work like a charm :) ","commit_id":"63b9fa56390796c93c76f3bb121015973663d9e7"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"3db6e431433dabf6d940c819e879145ebcd22880","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"433f0624_ed7d488f","updated":"2023-02-05 08:48:10.000000000","message":"recheck","commit_id":"63b9fa56390796c93c76f3bb121015973663d9e7"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"e1fa1117f0e4466664baa88c3a14094c1222265d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"daabcc79_0c658533","updated":"2023-02-08 10:27:08.000000000","message":"tested, thanks for spotting this.","commit_id":"63b9fa56390796c93c76f3bb121015973663d9e7"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"1d104b49b326832f79d1130c29835d2e30cf0361","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"3d44de21_e5960ff1","in_reply_to":"daabcc79_0c658533","updated":"2023-02-08 10:31:04.000000000","message":"Your welcome, I noticed this because heat-engines are down if you check heat-manage agents list (or something like that ). So I was investigating why - and found that this is global issue and every container is killed after 10 seconds. Which can result in various problems - specially during upgrades of critical components as rabbitmq and mariadb ..etc...","commit_id":"63b9fa56390796c93c76f3bb121015973663d9e7"}],"ansible/library/kolla_docker.py":[{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"9c02df2de20e3f2238c5f5cff51c4282d374b59e","unresolved":true,"context_lines":[{"line_number":354,"context_line":"    for key, value in module.params.items():"},{"line_number":355,"context_line":"        if key in new_args and value is None:"},{"line_number":356,"context_line":"            continue"},{"line_number":357,"context_line":"        if key in new_args and new_args[key] is not None:"},{"line_number":358,"context_line":"            continue"},{"line_number":359,"context_line":"        new_args[key] \u003d value"},{"line_number":360,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"2a590071_d5c244f2","line":357,"updated":"2023-01-18 09:47:18.000000000","message":"This suggests common_options takes precedence over module parameters? That seems wrong to me.\n\nThe precedence should be:\n\nprovided module arguments \u003e common options \u003e module parameter defaults\n\nI think the problem with the current logic is that if a parameter default is not None, then it overrides any common options.\n\nProbably the \"correct\" way to do this is using module defaults https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_module_defaults.html. It would make site.yml more verbose though.","commit_id":"70d68962364e47072df82d38f93319aa39cf99fa"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"0d03c1b321b4ed3eeb6ec972118f8218d72d8a00","unresolved":true,"context_lines":[{"line_number":354,"context_line":"    for key, value in module.params.items():"},{"line_number":355,"context_line":"        if key in new_args and value is None:"},{"line_number":356,"context_line":"            continue"},{"line_number":357,"context_line":"        if key in new_args and new_args[key] is not None:"},{"line_number":358,"context_line":"            continue"},{"line_number":359,"context_line":"        new_args[key] \u003d value"},{"line_number":360,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"619bf3fe_934cb8fd","line":357,"in_reply_to":"26e6e3ad_69c0c9c7","updated":"2023-01-18 12:20:23.000000000","message":"\u003e Hmm, that sounds nice, but then we can drop common options, can\u0027t we ?\n\nIt could be dropped from module arguments, yes.\n\n\u003e This will be XL patchset ... I really don\u0027t like it ..i better fix the logic in kolla_docker.py file ..what do you think ?\n\nIs that using module_defaults for every module? If we just do it to fix this issue it should be 2 extra lines per play, and lots of single line common_options argument removals.\n\nI\u0027m open to a python fix, I just couldn\u0027t see how to do it in a \"nice\" way when there are defaults that are not None.","commit_id":"70d68962364e47072df82d38f93319aa39cf99fa"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"352d1eb351fa71560cbfd4c669467195d1565675","unresolved":true,"context_lines":[{"line_number":354,"context_line":"    for key, value in module.params.items():"},{"line_number":355,"context_line":"        if key in new_args and value is None:"},{"line_number":356,"context_line":"            continue"},{"line_number":357,"context_line":"        if key in new_args and new_args[key] is not None:"},{"line_number":358,"context_line":"            continue"},{"line_number":359,"context_line":"        new_args[key] \u003d value"},{"line_number":360,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"2c083914_cfc06814","line":357,"in_reply_to":"2a590071_d5c244f2","updated":"2023-01-18 10:30:39.000000000","message":"Hmm, that sounds nice, but then we can drop common options, can\u0027t we ?","commit_id":"70d68962364e47072df82d38f93319aa39cf99fa"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"1357a8a69d4c7198fa0bf932180924523e6f88c0","unresolved":true,"context_lines":[{"line_number":354,"context_line":"    for key, value in module.params.items():"},{"line_number":355,"context_line":"        if key in new_args and value is None:"},{"line_number":356,"context_line":"            continue"},{"line_number":357,"context_line":"        if key in new_args and new_args[key] is not None:"},{"line_number":358,"context_line":"            continue"},{"line_number":359,"context_line":"        new_args[key] \u003d value"},{"line_number":360,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"26e6e3ad_69c0c9c7","line":357,"in_reply_to":"2c083914_cfc06814","updated":"2023-01-18 11:19:33.000000000","message":"Ok, I agree you are right about the precedence ..but this can be rewriten in kolla_docker.py file instead of rewriting all ansible playbooks/roles ...etc. A while ago I reimplemented to module_defaults and it is something like below : \n\nmichalarbet@pixla:~/ultimum/git/upstream/kolla-ansible$ echo \"deleted_lines \u003d $(git diff | grep \u0027^\\-\u0027 | wc -l )\" ; echo \"added_lines \u003d $(git diff | grep \u0027^\\+\u0027 | wc -l )\"; echo \"changed_files \u003d $(git diff --stat | wc -l )\"\ndeleted_lines \u003d 600\nadded_lines \u003d 1022\nchanged_files \u003d 198\n\n\nThis will be XL patchset ... I really don\u0027t like it ..i better fix the logic in kolla_docker.py file ..what do you think ?","commit_id":"70d68962364e47072df82d38f93319aa39cf99fa"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"3a1e685ad449595259807ff8d7252a63c29e4a17","unresolved":false,"context_lines":[{"line_number":354,"context_line":"    for key, value in module.params.items():"},{"line_number":355,"context_line":"        if key in new_args and value is None:"},{"line_number":356,"context_line":"            continue"},{"line_number":357,"context_line":"        if key in new_args and new_args[key] is not None:"},{"line_number":358,"context_line":"            continue"},{"line_number":359,"context_line":"        new_args[key] \u003d value"},{"line_number":360,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"aaadcee3_9f2e8c74","line":357,"in_reply_to":"619bf3fe_934cb8fd","updated":"2023-01-18 14:56:53.000000000","message":"Done","commit_id":"70d68962364e47072df82d38f93319aa39cf99fa"}],"tests/link-module-utils.sh":[{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"eb1bf53d3a0b24007b22274b4e59008436a450d4","unresolved":true,"context_lines":[{"line_number":7,"context_line":""},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"local_module_utils\u003d${1}/ansible/module_utils"},{"line_number":10,"context_line":"env_module_utils\u003d$(/usr/bin/env python -c \"import ansible; print(ansible.__path__[0] + \u0027/module_utils\u0027)\")"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"for file_path in ${local_module_utils}/*.py; do"},{"line_number":13,"context_line":"    file_name\u003d$(basename ${file_path})"}],"source_content_type":"text/x-sh","patch_set":15,"id":"eb3b7509_30b696ff","line":10,"range":{"start_line":10,"start_character":0,"end_line":10,"end_character":105},"updated":"2023-02-06 10:25:08.000000000","message":"why? not mentioned in the commit message even","commit_id":"63b9fa56390796c93c76f3bb121015973663d9e7"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"07244ee90fb6b8cfe788168ab7738b3f980197d4","unresolved":true,"context_lines":[{"line_number":7,"context_line":""},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"local_module_utils\u003d${1}/ansible/module_utils"},{"line_number":10,"context_line":"env_module_utils\u003d$(/usr/bin/env python -c \"import ansible; print(ansible.__path__[0] + \u0027/module_utils\u0027)\")"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"for file_path in ${local_module_utils}/*.py; do"},{"line_number":13,"context_line":"    file_name\u003d$(basename ${file_path})"}],"source_content_type":"text/x-sh","patch_set":15,"id":"fb6871b0_75852706","line":10,"range":{"start_line":10,"start_character":0,"end_line":10,"end_character":105},"in_reply_to":"eb3b7509_30b696ff","updated":"2023-02-06 15:05:44.000000000","message":"Ok, when you run tox -e py310 on ubuntu you will see something similar below : \n\nmichalarbet@pixla:/tmp/kolla-ansible$ tox -e py310\npy310 create: /tmp/kolla-ansible/.tox/py310\npy310 installdeps: -chttps://releases.openstack.org/constraints/upper/master, -r/tmp/kolla-ansible/requirements.txt, -r/tmp/kolla-ansible/test-requirements.txt\npy310 develop-inst: /tmp/kolla-ansible\npy310 installed: ansible\u003d\u003d5.10.0,ansible-core\u003d\u003d2.12.10,attrs\u003d\u003d22.1.0,autopage\u003d\u003d0.5.1,certifi\u003d\u003d2022.12.7,cffi\u003d\u003d1.15.1,charset-normalizer\u003d\u003d2.1.1,cliff\u003d\u003d4.1.0,cmd2\u003d\u003d2.4.2,coverage\u003d\u003d6.5.0,cryptography\u003d\u003d38.0.2,debtcollector\u003d\u003d2.5.0,docker\u003d\u003d6.0.0,extras\u003d\u003d1.0.0,fixtures\u003d\u003d4.0.1,future\u003d\u003d0.18.2,hvac\u003d\u003d1.0.2,idna\u003d\u003d3.4,importlib-metadata\u003d\u003d5.0.0,iso8601\u003d\u003d1.1.0,Jinja2\u003d\u003d3.1.2,jmespath\u003d\u003d1.0.1,-e git+https://github.com/openstack/kolla-ansible.git@f253f99c1216705cc52e65f6229f8ae42bfda178#egg\u003dkolla_ansible,MarkupSafe\u003d\u003d2.1.1,netaddr\u003d\u003d0.8.0,netifaces\u003d\u003d0.11.0,oslo.config\u003d\u003d9.1.0,oslo.i18n\u003d\u003d5.1.0,oslo.utils\u003d\u003d6.1.0,oslotest\u003d\u003d4.5.0,packaging\u003d\u003d21.3,pbr\u003d\u003d5.11.1,prettytable\u003d\u003d3.4.1,pycparser\u003d\u003d2.21,pyhcl\u003d\u003d0.4.4,pyparsing\u003d\u003d3.0.9,pyperclip\u003d\u003d1.8.2,python-subunit\u003d\u003d1.4.0,pytz\u003d\u003d2022.4,PyYAML\u003d\u003d6.0,requests\u003d\u003d2.28.1,resolvelib\u003d\u003d0.5.4,rfc3986\u003d\u003d1.5.0,six\u003d\u003d1.16.0,stestr\u003d\u003d4.0.1,stevedore\u003d\u003d4.1.1,testtools\u003d\u003d2.5.0,urllib3\u003d\u003d1.26.12,voluptuous\u003d\u003d0.13.1,wcwidth\u003d\u003d0.2.5,websocket-client\u003d\u003d1.4.1,wrapt\u003d\u003d1.14.1,zipp\u003d\u003d3.8.1\npy310 run-test-pre: PYTHONHASHSEED\u003d\u00271813218598\u0027\npy310 run-test: commands[0] | find . -type f -name \u0027*.py[c|o]\u0027 -delete -o -type l -name \u0027*.py[c|o]\u0027 -delete\npy310 run-test: commands[1] | find . -type d -name __pycache__ -delete\npy310 run-test: commands[2] | bash /tmp/kolla-ansible/tests/link-module-utils.sh /tmp/kolla-ansible /tmp/kolla-ansible/.tox/py310/local/lib/python3.10/dist-packages\nrealpath: /tmp/kolla-ansible/.tox/py310/local/lib/python3.10/dist-packages/ansible/module_utils: No such file or directory\nln: failed to create symbolic link \u0027/kolla_docker_worker.py\u0027: Permission denied\nrealpath: /tmp/kolla-ansible/.tox/py310/local/lib/python3.10/dist-packages/ansible/module_utils: No such file or directory\nln: failed to create symbolic link \u0027/kolla_systemd_worker.py\u0027: Permission denied\nERROR: InvocationError for command /usr/bin/bash tests/link-module-utils.sh . /tmp/kolla-ansible/.tox/py310/local/lib/python3.10/dist-packages (exited with code 1)\n_____________________________________________________________________________________________________ summary _____________________________________________________________________________________________________\nERROR:   py310: commands failed\n\n\nPath is resolved different, so fix is just to get path of ansible in tox venv, nothing less, nothing more.","commit_id":"63b9fa56390796c93c76f3bb121015973663d9e7"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"c2dc31c6282d8227a8ca8730f69aca54b613ae5c","unresolved":false,"context_lines":[{"line_number":7,"context_line":""},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"local_module_utils\u003d${1}/ansible/module_utils"},{"line_number":10,"context_line":"env_module_utils\u003d$(/usr/bin/env python -c \"import ansible; print(ansible.__path__[0] + \u0027/module_utils\u0027)\")"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"for file_path in ${local_module_utils}/*.py; do"},{"line_number":13,"context_line":"    file_name\u003d$(basename ${file_path})"}],"source_content_type":"text/x-sh","patch_set":15,"id":"532653ae_20909b14","line":10,"range":{"start_line":10,"start_character":0,"end_line":10,"end_character":105},"in_reply_to":"fb6871b0_75852706","updated":"2023-02-07 08:43:36.000000000","message":"Done","commit_id":"63b9fa56390796c93c76f3bb121015973663d9e7"}],"tests/test_kolla_docker.py":[{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"eb1bf53d3a0b24007b22274b4e59008436a450d4","unresolved":true,"context_lines":[{"line_number":254,"context_line":"        \u0027client_timeout\u0027: 120,"},{"line_number":255,"context_line":"    }"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"    new_args \u003d module.params.pop(\u0027common_options\u0027, dict()) or dict()"},{"line_number":258,"context_line":"    env_module_environment \u003d module.params.pop(\u0027environment\u0027, dict()) or dict()"},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"    for k, v in module.params.items():"},{"line_number":261,"context_line":"        if v is None:"},{"line_number":262,"context_line":"            if k in common_options_defaults:"},{"line_number":263,"context_line":"                if k in new_args:"},{"line_number":264,"context_line":"                    # From ansible groups vars the common options"},{"line_number":265,"context_line":"                    # can be string or int"},{"line_number":266,"context_line":"                    if isinstance(new_args[k], str) and new_args[k].isdigit():"},{"line_number":267,"context_line":"                        new_args[k] \u003d int(new_args[k])"},{"line_number":268,"context_line":"                    continue"},{"line_number":269,"context_line":"                else:"},{"line_number":270,"context_line":"                    if common_options_defaults[k] is not None:"},{"line_number":271,"context_line":"                        new_args[k] \u003d common_options_defaults[k]"},{"line_number":272,"context_line":"            else:"},{"line_number":273,"context_line":"                continue"},{"line_number":274,"context_line":"        if v is not None:"},{"line_number":275,"context_line":"            new_args[k] \u003d v"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    env_module_common_options \u003d new_args.pop(\u0027environment\u0027, dict())"},{"line_number":278,"context_line":"    new_args[\u0027environment\u0027] \u003d env_module_common_options"},{"line_number":279,"context_line":"    new_args[\u0027environment\u0027].update(env_module_environment)"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"    # if pid_mode \u003d \"\"/None/False, remove it"},{"line_number":282,"context_line":"    if not new_args.get(\u0027pid_mode\u0027, False):"},{"line_number":283,"context_line":"        new_args.pop(\u0027pid_mode\u0027, None)"},{"line_number":284,"context_line":"    # if ipc_mode \u003d \"\"/None/False, remove it"},{"line_number":285,"context_line":"    if not new_args.get(\u0027ipc_mode\u0027, False):"},{"line_number":286,"context_line":"        new_args.pop(\u0027ipc_mode\u0027, None)"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"    module.params \u003d new_args"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    with mock.patch(\"docker.APIClient\") as MockedDockerClientClass:"},{"line_number":291,"context_line":"        MockedDockerClientClass.return_value._version \u003d docker_api_version"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa01d2c5_0ed38e2e","line":288,"range":{"start_line":257,"start_character":0,"end_line":288,"end_character":28},"updated":"2023-02-06 10:25:08.000000000","message":"Is there an option we don\u0027t copy this from the original code?","commit_id":"63b9fa56390796c93c76f3bb121015973663d9e7"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"07244ee90fb6b8cfe788168ab7738b3f980197d4","unresolved":false,"context_lines":[{"line_number":254,"context_line":"        \u0027client_timeout\u0027: 120,"},{"line_number":255,"context_line":"    }"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"    new_args \u003d module.params.pop(\u0027common_options\u0027, dict()) or dict()"},{"line_number":258,"context_line":"    env_module_environment \u003d module.params.pop(\u0027environment\u0027, dict()) or dict()"},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"    for k, v in module.params.items():"},{"line_number":261,"context_line":"        if v is None:"},{"line_number":262,"context_line":"            if k in common_options_defaults:"},{"line_number":263,"context_line":"                if k in new_args:"},{"line_number":264,"context_line":"                    # From ansible groups vars the common options"},{"line_number":265,"context_line":"                    # can be string or int"},{"line_number":266,"context_line":"                    if isinstance(new_args[k], str) and new_args[k].isdigit():"},{"line_number":267,"context_line":"                        new_args[k] \u003d int(new_args[k])"},{"line_number":268,"context_line":"                    continue"},{"line_number":269,"context_line":"                else:"},{"line_number":270,"context_line":"                    if common_options_defaults[k] is not None:"},{"line_number":271,"context_line":"                        new_args[k] \u003d common_options_defaults[k]"},{"line_number":272,"context_line":"            else:"},{"line_number":273,"context_line":"                continue"},{"line_number":274,"context_line":"        if v is not None:"},{"line_number":275,"context_line":"            new_args[k] \u003d v"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    env_module_common_options \u003d new_args.pop(\u0027environment\u0027, dict())"},{"line_number":278,"context_line":"    new_args[\u0027environment\u0027] \u003d env_module_common_options"},{"line_number":279,"context_line":"    new_args[\u0027environment\u0027].update(env_module_environment)"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"    # if pid_mode \u003d \"\"/None/False, remove it"},{"line_number":282,"context_line":"    if not new_args.get(\u0027pid_mode\u0027, False):"},{"line_number":283,"context_line":"        new_args.pop(\u0027pid_mode\u0027, None)"},{"line_number":284,"context_line":"    # if ipc_mode \u003d \"\"/None/False, remove it"},{"line_number":285,"context_line":"    if not new_args.get(\u0027ipc_mode\u0027, False):"},{"line_number":286,"context_line":"        new_args.pop(\u0027ipc_mode\u0027, None)"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"    module.params \u003d new_args"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    with mock.patch(\"docker.APIClient\") as MockedDockerClientClass:"},{"line_number":291,"context_line":"        MockedDockerClientClass.return_value._version \u003d docker_api_version"}],"source_content_type":"text/x-python","patch_set":15,"id":"8305ccb4_835ee62b","line":288,"range":{"start_line":257,"start_character":0,"end_line":288,"end_character":28},"in_reply_to":"fa01d2c5_0ed38e2e","updated":"2023-02-06 15:05:44.000000000","message":"No, that\u0027s the basic idea of writing a unit tests. Unit tests has to work the same way as the original code, and original code is same as unit test.\n\nIf you are asking for module.params \u003d copy.deepcopy(mod_param) , this is only  because mod_param is parameter ..not object as in kolla docker worker python script.\n\nIf you will not use copy.deepcopy ..code below is changing mod_param directly even if you assigned to module.params. Python is using reference.","commit_id":"63b9fa56390796c93c76f3bb121015973663d9e7"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"eb1bf53d3a0b24007b22274b4e59008436a450d4","unresolved":true,"context_lines":[{"line_number":360,"context_line":""},{"line_number":361,"context_line":"    def test_common_options_defaults(self):"},{"line_number":362,"context_line":"        self.dw \u003d get_DockerWorker(self.fake_data[\u0027params\u0027])"},{"line_number":363,"context_line":"        self.assertEqual(self.dw.params[\u0027api_version\u0027], \u0027auto\u0027)"},{"line_number":364,"context_line":"        self.assertEqual(self.dw.params[\u0027restart_retries\u0027], 10)"},{"line_number":365,"context_line":"        self.assertEqual(self.dw.params[\u0027graceful_timeout\u0027], 10)"},{"line_number":366,"context_line":"        self.assertEqual(self.dw.params[\u0027client_timeout\u0027], 120)"},{"line_number":367,"context_line":"        self.assertEqual(self.dw.params[\u0027environment\u0027], {})"},{"line_number":368,"context_line":"        self.assertNotIn(\u0027auth_email\u0027, self.dw.params)"},{"line_number":369,"context_line":"        self.assertNotIn(\u0027auth_password\u0027, self.dw.params)"},{"line_number":370,"context_line":"        self.assertNotIn(\u0027auth_registry\u0027, self.dw.params)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3cd0afc7_a5f472ed","line":367,"range":{"start_line":363,"start_character":0,"end_line":367,"end_character":59},"updated":"2023-02-06 10:25:08.000000000","message":"is there an option we don\u0027t reply this over three functions with hardcoded values (you already have the values defined earlier)?","commit_id":"63b9fa56390796c93c76f3bb121015973663d9e7"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"07244ee90fb6b8cfe788168ab7738b3f980197d4","unresolved":true,"context_lines":[{"line_number":360,"context_line":""},{"line_number":361,"context_line":"    def test_common_options_defaults(self):"},{"line_number":362,"context_line":"        self.dw \u003d get_DockerWorker(self.fake_data[\u0027params\u0027])"},{"line_number":363,"context_line":"        self.assertEqual(self.dw.params[\u0027api_version\u0027], \u0027auto\u0027)"},{"line_number":364,"context_line":"        self.assertEqual(self.dw.params[\u0027restart_retries\u0027], 10)"},{"line_number":365,"context_line":"        self.assertEqual(self.dw.params[\u0027graceful_timeout\u0027], 10)"},{"line_number":366,"context_line":"        self.assertEqual(self.dw.params[\u0027client_timeout\u0027], 120)"},{"line_number":367,"context_line":"        self.assertEqual(self.dw.params[\u0027environment\u0027], {})"},{"line_number":368,"context_line":"        self.assertNotIn(\u0027auth_email\u0027, self.dw.params)"},{"line_number":369,"context_line":"        self.assertNotIn(\u0027auth_password\u0027, self.dw.params)"},{"line_number":370,"context_line":"        self.assertNotIn(\u0027auth_registry\u0027, self.dw.params)"}],"source_content_type":"text/x-python","patch_set":15,"id":"4abba037_bcc78c1f","line":367,"range":{"start_line":363,"start_character":0,"end_line":367,"end_character":59},"in_reply_to":"3cd0afc7_a5f472ed","updated":"2023-02-06 15:05:44.000000000","message":"But every function is doing something different:\n\nTest_common_options_defaults -\u003e is testing if module (without defined module\u0027s parameter common_options) has defaults (as they were removed from module paameters defaults - but moved to dict with defaults).\n\ntest_common_options - is testing if module with common_options parameter defined (with *different values* than default) match different values in params \n\nHere assert can be changed to mapping to FAKE_DATA_COMMON_OPTS, if they are same.\n\ntest_common_options_overriden - this is test if parameter has priority before common_options.","commit_id":"63b9fa56390796c93c76f3bb121015973663d9e7"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"c2dc31c6282d8227a8ca8730f69aca54b613ae5c","unresolved":false,"context_lines":[{"line_number":360,"context_line":""},{"line_number":361,"context_line":"    def test_common_options_defaults(self):"},{"line_number":362,"context_line":"        self.dw \u003d get_DockerWorker(self.fake_data[\u0027params\u0027])"},{"line_number":363,"context_line":"        self.assertEqual(self.dw.params[\u0027api_version\u0027], \u0027auto\u0027)"},{"line_number":364,"context_line":"        self.assertEqual(self.dw.params[\u0027restart_retries\u0027], 10)"},{"line_number":365,"context_line":"        self.assertEqual(self.dw.params[\u0027graceful_timeout\u0027], 10)"},{"line_number":366,"context_line":"        self.assertEqual(self.dw.params[\u0027client_timeout\u0027], 120)"},{"line_number":367,"context_line":"        self.assertEqual(self.dw.params[\u0027environment\u0027], {})"},{"line_number":368,"context_line":"        self.assertNotIn(\u0027auth_email\u0027, self.dw.params)"},{"line_number":369,"context_line":"        self.assertNotIn(\u0027auth_password\u0027, self.dw.params)"},{"line_number":370,"context_line":"        self.assertNotIn(\u0027auth_registry\u0027, self.dw.params)"}],"source_content_type":"text/x-python","patch_set":15,"id":"a1012ae4_851442ec","line":367,"range":{"start_line":363,"start_character":0,"end_line":367,"end_character":59},"in_reply_to":"4abba037_bcc78c1f","updated":"2023-02-07 08:43:36.000000000","message":"Done","commit_id":"63b9fa56390796c93c76f3bb121015973663d9e7"}]}
