)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14200,"name":"Maksim Malchuk","email":"maksim.malchuk@gmail.com","username":"mmalchuk"},"change_message_id":"575a1cd7f8b505abfa1e050ef35b5178e96f7404","unresolved":true,"context_lines":[{"line_number":10,"context_line":"files in set_configs.py, introducing functions for managing"},{"line_number":11,"context_line":"default configuration files first."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"It also updates the logging configuration to use a more"},{"line_number":14,"context_line":"detailed format."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: #2060855"},{"line_number":17,"context_line":"Change-Id: If91e0330dc149143c82d2183b8ddf6fa9f68d80e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":22,"id":"645485b0_1d47cd53","line":14,"range":{"start_line":13,"start_character":0,"end_line":14,"end_character":16},"updated":"2024-11-06 09:12:50.000000000","message":"are not this moved to the separate change?","commit_id":"39a337399c6deca83b98303bc67098792b546095"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"deda4b12e2a7515dac931e7040c879895811f87f","unresolved":false,"context_lines":[{"line_number":10,"context_line":"files in set_configs.py, introducing functions for managing"},{"line_number":11,"context_line":"default configuration files first."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"It also updates the logging configuration to use a more"},{"line_number":14,"context_line":"detailed format."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: #2060855"},{"line_number":17,"context_line":"Change-Id: If91e0330dc149143c82d2183b8ddf6fa9f68d80e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":22,"id":"2cb09d8d_5f88afec","line":14,"range":{"start_line":13,"start_character":0,"end_line":14,"end_character":16},"in_reply_to":"2330d04a_75cacba5","updated":"2024-11-06 12:55:49.000000000","message":"Done","commit_id":"39a337399c6deca83b98303bc67098792b546095"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"5db7512c5f3ff61ab1c1f621165cc786e16ecf7e","unresolved":true,"context_lines":[{"line_number":10,"context_line":"files in set_configs.py, introducing functions for managing"},{"line_number":11,"context_line":"default configuration files first."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"It also updates the logging configuration to use a more"},{"line_number":14,"context_line":"detailed format."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: #2060855"},{"line_number":17,"context_line":"Change-Id: If91e0330dc149143c82d2183b8ddf6fa9f68d80e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":22,"id":"2330d04a_75cacba5","line":14,"range":{"start_line":13,"start_character":0,"end_line":14,"end_character":16},"in_reply_to":"645485b0_1d47cd53","updated":"2024-11-06 09:36:41.000000000","message":"good catch, yes.","commit_id":"39a337399c6deca83b98303bc67098792b546095"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":14200,"name":"Maksim Malchuk","email":"maksim.malchuk@gmail.com","username":"mmalchuk"},"change_message_id":"ded94ee548a071ed82784a37c8b55e7962013f42","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"cee8cfcd_1f0b28ba","updated":"2024-04-10 22:11:51.000000000","message":"Shouldn\u0027t we have a pice of documentation about /etc/kolla/defaults and it contents? Maybe described logic too?","commit_id":"20c7f7983c490f8db202b8837a88c9b7846a9ac6"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"b6761fe7119746fafa1b06b7ad51e4709c8cb170","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8e508473_e045dfa3","updated":"2024-04-10 18:25:13.000000000","message":"That\u0027s going to work only on a fresh deployment, right?\nIf you have running containers - the defaults that will get copied - are the ones kolla copied into /etc/$service_dir?","commit_id":"20c7f7983c490f8db202b8837a88c9b7846a9ac6"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"135859f98c6e8962750eb63b9ae7e87072fc9577","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"932062b5_f5dc4d5f","updated":"2024-04-10 20:57:26.000000000","message":"hmm, grafana build failing ? ok, I\u0027ve seen some discussion ...but locally working ...","commit_id":"20c7f7983c490f8db202b8837a88c9b7846a9ac6"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"950d5185edb05e5141f2ad1f841b5ee125c94184","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e66d1b5f_8c695ec9","in_reply_to":"1e74b3c9_d9a79c15","updated":"2024-04-11 07:28:44.000000000","message":"Done","commit_id":"20c7f7983c490f8db202b8837a88c9b7846a9ac6"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"a91d6e5858b6c9c4ff11d78e0f1d886ec26bc04b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"84bca5e8_03dd1d5d","in_reply_to":"2674aa4f_9adcf050","updated":"2024-04-11 11:33:03.000000000","message":"I find this extremely clunky and messy, this can’t be the best solution.","commit_id":"20c7f7983c490f8db202b8837a88c9b7846a9ac6"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"b471bc8606746128d11ec9a0e2855f1972ed0894","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f6d91ccc_990b74e5","in_reply_to":"84bca5e8_03dd1d5d","updated":"2024-04-11 19:46:05.000000000","message":"which part ? I would appreciate specific feedback and reasons why it\u0027s messy and clunky. Let\u0027s discuss, rather than expressing only feelings.\n\nIf you think it\u0027s","commit_id":"20c7f7983c490f8db202b8837a88c9b7846a9ac6"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"35e197f04274a526e85f7ad220acfc5eb06b1991","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"2674aa4f_9adcf050","in_reply_to":"8e508473_e045dfa3","updated":"2024-04-10 18:55:18.000000000","message":"Well, yes, exactly as you said ... as you have to create a state once ....\n\nBut, I can add to reno a side note that if users want to fix it completly, they need to recreate container... \n\nOR just to inform them that it will be fixed with new image TAG .... so during the upgrade for example ...","commit_id":"20c7f7983c490f8db202b8837a88c9b7846a9ac6"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"d756aaae076d5d38a7d4745a381ffa6d65c0cd92","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1e74b3c9_d9a79c15","in_reply_to":"cee8cfcd_1f0b28ba","updated":"2024-04-11 07:27:58.000000000","message":"Really ? are we going to document every piece of code ? This code is used internally, nothing what user should know about...","commit_id":"20c7f7983c490f8db202b8837a88c9b7846a9ac6"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"75bf8d2e735ff4554f7c8702a627f99cd3e15f12","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"409601fa_8445f8b9","in_reply_to":"f6d91ccc_990b74e5","updated":"2024-04-12 14:28:51.000000000","message":"Reworked","commit_id":"20c7f7983c490f8db202b8837a88c9b7846a9ac6"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"91219819a5ff85fa2f8dfd6e7cf4990ab3bd0d7f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"84874daf_7c77c6bc","updated":"2024-04-17 12:43:05.000000000","message":"Actually - if the bug is around policy.yaml/json - why don\u0027t we introduce state: absent in the config.json framework and remove such files?\nIsn\u0027t that more simplistic approach? Would it work for everything? (e.g. api-paste.ini files?)","commit_id":"cb233d1ed0f38043978322ec697f07f18b73611c"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"97292f154767771ddaa7ab072b8db5c805f91714","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"e3301924_87519608","in_reply_to":"84874daf_7c77c6bc","updated":"2024-04-30 10:40:15.000000000","message":"Actually this is not bug about policy.yaml/json only, this is bug about any file injected into container and no way how to restore the original file.\n\nLet\u0027s say i will create /etc/kolla/{service}/apache2.conf with \n\necho \u0027this is not about policy.json/yaml only\u0027 \u003e  /etc/kolla/{service}/apache2.conf\n\nI will manually edit /etc/kolla/{service}/config.json and i will add entry for copying apache2.conf\n\nAfter restart container, I have not-working apache2 configuration, totally broken.... and even if i revert back changes in config.json, from moment the container was restarted ..there will be the copied file ...no way how to restore.\n\nWhat new proposed functions does is only : \n\n1. Before copying file, check if the file exists in path\n2. If exist, backup it and mark in state that file existed and backup created\n3. Copy file\n\nand vice versa : \n\n1. Check and apply state \n2. Then copy everything from /var/lib/kolla/config_files...\n\n\nYes it will work for everything, did you review the code ? There isn\u0027t any condiditonal for some filenames or something ... Actually that code is very simple...","commit_id":"cb233d1ed0f38043978322ec697f07f18b73611c"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"110c68d0ee8f2cd249cc54443fd23fe0d8c09162","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"6577f2a9_24fdaba8","updated":"2024-06-27 16:27:52.000000000","message":"Hey, what about this ?","commit_id":"5a68f863e6b21e13bbb25dcfee8b1dfcf9fdc392"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"fe5619902f869cd69e7dd542ebc7b2e13e22756f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"acf32970_b4460b0f","updated":"2024-06-27 16:28:05.000000000","message":"Please, can we move on ?","commit_id":"5a68f863e6b21e13bbb25dcfee8b1dfcf9fdc392"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"74d052a5252711be8d9e3aef2809a16309452e00","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"1d571388_c31ebddd","updated":"2024-09-18 10:23:54.000000000","message":"\nFile\tfunction\tstatements\tmissing\texcluded\tcoverage\ndocker/base/set_configs.py\tExitingException.__init__\t2\t0\t0\t100%\ndocker/base/set_configs.py\tConfigFile.__init__\t7\t0\t0\t100%\ndocker/base/set_configs.py\tConfigFile.__str__\t1\t1\t0\t0%\ndocker/base/set_configs.py\tConfigFile._copy_file\t7\t7\t0\t0%\ndocker/base/set_configs.py\tConfigFile._merge_directories\t11\t9\t0\t18%\ndocker/base/set_configs.py\tConfigFile._delete_path\t6\t0\t0\t100%\ndocker/base/set_configs.py\tConfigFile._create_parent_dirs\t3\t3\t0\t0%\ndocker/base/set_configs.py\tConfigFile._set_properties\t3\t3\t0\t0%\ndocker/base/set_configs.py\tConfigFile._set_properties_from_file\t4\t0\t0\t100%\ndocker/base/set_configs.py\tConfigFile._set_properties_from_conf\t2\t0\t0\t100%\ndocker/base/set_configs.py\tConfigFile.copy\t17\t3\t0\t82%\ndocker/base/set_configs.py\tConfigFile._cmp_file\t23\t8\t0\t65%\ndocker/base/set_configs.py\tConfigFile._cmp_dir\t15\t15\t0\t0%\ndocker/base/set_configs.py\tConfigFile.check\t18\t0\t0\t100%\ndocker/base/set_configs.py\tvalidate_config\t9\t6\t0\t33%\ndocker/base/set_configs.py\tvalidate_source\t8\t8\t0\t0%\ndocker/base/set_configs.py\tload_config\t5\t0\t0\t100%\ndocker/base/set_configs.py\tload_config.load_from_file\t9\t4\t0\t56%\ndocker/base/set_configs.py\tcopy_config\t15\t3\t0\t80%\ndocker/base/set_configs.py\tuser_group\t6\t3\t0\t50%\ndocker/base/set_configs.py\thandle_permissions\t19\t19\t0\t0%\ndocker/base/set_configs.py\thandle_permissions.set_perms\t23\t23\t0\t0%\ndocker/base/set_configs.py\thandle_permissions.handle_exclusion\t7\t7\t0\t0%\ndocker/base/set_configs.py\tget_defaults_state\t5\t0\t0\t100%\ndocker/base/set_configs.py\tset_defaults_state\t2\t0\t0\t100%\ndocker/base/set_configs.py\tremove_or_restore_configs\t9\t0\t0\t100%\ndocker/base/set_configs.py\tbackup_configs\t18\t0\t0\t100%\ndocker/base/set_configs.py\thandle_defaults\t4\t0\t0\t100%\ndocker/base/set_configs.py\texecute_config_strategy\t14\t14\t0\t0%\ndocker/base/set_configs.py\texecute_config_check\t3\t3\t0\t0%\ndocker/base/set_configs.py\tmain\t15\t15\t0\t0%\ndocker/base/set_configs.py\t(no function)\t58\t1\t0\t98%\nTotal\t \t348\t155\t0\t55%","commit_id":"285ad6d11ae9d06c37a8bcccf9454ec7b79882f4"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"c2256ae3485c77d3cf9905e77a13395cb138ebe5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"fae33e2e_cd0e60e8","updated":"2024-09-18 10:23:07.000000000","message":"Now completly covered by unit tests.... 100 % And tested manually and working.\n\n\n```\u003e def get_defaults_state():\n\u003e     \"\"\"Retrieve the saved default configuration state from default state file.\n\n\u003e     This function creates the directory for Kolla defaults if it does not\n\u003e     exist, and then attempts to read the current configuration state from\n\u003e     a JSON file. If the file exists, it reads and returns the content.\n\u003e     If not, it returns an empty dictionary.\n\n\u003e     Simply said, when the container starts for the first time, the state file\n\u003e     doesn\u0027t exist, and it returns an empty dictionary.\n\u003e     However, if it has already been started before, it will contain the state\n\u003e     as it was when it first ran.\n\n\u003e     Returns:\n\u003e         dict: The configuration state stored in the Kolla defaults state file.\n\n\u003e     Example:\n\u003e         {\n\u003e             \"/etc/cinder/cinder.conf\": {\n\u003e                 \"source\": \"/etc/cinder/cinder.conf\",\n\u003e                 \"preserve_properties\": true,\n\u003e                 \"dest\": null\n\u003e             },\n\u003e             \"/etc/apache2/conf-enabled/cinder-wsgi.conf\": {\n\u003e                 \"source\": \"/etc/apache2/conf-enabled/cinder-wsgi.conf\",\n\u003e                 \"preserve_properties\": true,\n\u003e                 \"dest\": null\n\u003e             },\n\u003e             \"/etc/cinder/cinder_audit_map.conf\": {\n\u003e                 \"source\": \"/etc/cinder/cinder_audit_map.conf\",\n\u003e                 \"preserve_properties\": true,\n\u003e                 \"dest\": \"/etc/kolla/defaults/etc/cinder/cinder_audit_map.conf\"\n\u003e             }\n\u003e         }\n\n\u003e         From above example:\n\u003e         /etc/cinder/cinder.conf didn\u0027t exist\n\u003e         /etc/apache2/conf-enabled/cinder-wsgi.conf didn\u0027t exist\n\u003e         /etc/cinder/cinder_audit_map.conf exists and saved\n\u003e     \"\"\"\n\u003e     os.makedirs(KOLLA_DEFAULTS, exist_ok\u003dTrue)\n\u003e     if os.path.exists(KOLLA_DEFAULTS_STATE):\n\u003e         with open(KOLLA_DEFAULTS_STATE, \u0027r\u0027) as f:\n\u003e             return json.load(f)\n\u003e     else:\n\u003e         return {}\n\n\u003e def set_defaults_state(state):\n\u003e     \"\"\"Save the provided configuration state to the defaults state file.\n\n\u003e     This function writes the provided state (a dictionary) to a JSON file at\n\u003e     the specified Kolla defaults state location, ensuring that it is properly\n\u003e     formatted with indentation for readability.\n\n\u003e     Args:\n\u003e         state (dict): The configuration state to save to the Kolla defaults\n\u003e         state file.\n\u003e     \"\"\"\n\u003e     with open(KOLLA_DEFAULTS_STATE, \u0027w\u0027) as f:\n\u003e         json.dump(state, f, indent\u003d4)\n\n\n\u003e def remove_or_restore_configs(state):\n\u003e     for k, v in state.items():\n\u003e         if v[\u0027dest\u0027] is None:\n\u003e             if os.path.exists(k):\n\u003e                 if os.path.isfile(k):\n\u003e                     os.remove(k)\n\u003e                 else:\n\u003e                     shutil.rmtree(k)\n\u003e         else:\n\u003e             v[\u0027source\u0027], v[\u0027dest\u0027] \u003d v[\u0027dest\u0027], v[\u0027source\u0027]\n\u003e             config_file \u003d ConfigFile(**v)\n\u003e             config_file.copy()\n\n\u003e def backup_configs(config, state):\n\u003e     if \u0027config_files\u0027 in config:\n\u003e         for data in config[\u0027config_files\u0027]:\n\u003e             if data[\u0027dest\u0027] in state.keys():\n\u003e                 continue\n\u003e             src \u003d data[\u0027source\u0027]\n\u003e             dst \u003d data[\u0027dest\u0027]\n\u003e             default \u003d KOLLA_DEFAULTS + dst\n\u003e             if os.path.exists(src):\n\u003e                 copy \u003d {\u0027source\u0027: dst, \u0027preserve_properties\u0027: True}\n\u003e                 if os.path.exists(dst):\n\u003e                     copy[\u0027dest\u0027] \u003d default\n\u003e                     if dst not in state:\n\u003e                         config_file \u003d ConfigFile(**copy)\n\u003e                         config_file.copy() \n\u003e                         state[dst] \u003d copy\n\u003e                 else:\n\u003e                     copy[\u0027dest\u0027] \u003d None\n\u003e                     if dst not in state:\n\u003e                         state[dst] \u003d copy\n\n\u003e def handle_defaults(config):\n\u003e     \"\"\"Handle the default configuration files by copy/remove them as needed\n      \n\u003e     This function loads the current default state and iterates over the\n\u003e     configuration files. If the file or directory exists and is part of the\n\u003e     current state, it is copied or removed based on its destination. If new\n\u003e     configuration files are provided in the input config,\n\u003e     they are added to the default state and processed accordingly.\n\n\u003e     Args:\n\u003e         config (dict): The configuration containing the list of config files\n\u003e         to be handled.\n\u003e     \"\"\"\n\u003e     state \u003d get_defaults_state()\n\u003e     remove_or_restore_configs(state)\n\u003e     backup_configs(config, state)\n\u003e     set_defaults_state(state)```","commit_id":"285ad6d11ae9d06c37a8bcccf9454ec7b79882f4"},{"author":{"_account_id":32398,"name":"Gaël THEROND","display_name":"Fl1nt","email":"gael.therond@bitswalk.com","username":"Fl1nt"},"change_message_id":"5e9f67af0cb2b66a6b9a21fa0b689c69a0edd714","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"6905196f_2a3f9f9a","updated":"2024-09-26 14:05:33.000000000","message":"LGTM, nothing special to add.","commit_id":"0d9aa2b311595743782eb49d4d185df75ae38f0c"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"a28d9af1db4771c565ea35a9cf1c8176ec2b3075","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"3d662e22_6e2647bd","updated":"2024-10-29 09:16:03.000000000","message":"What are we waiting for? All reviewers are satisfied now, no comments left; This patch just need the second +2 and +w","commit_id":"1796e6d0f26d7899fb05f19d04978271390d44e4"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"cb97841d1157412827d99793bfade0c8b7f2037a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"28eef69a_6c427ec4","updated":"2024-10-09 19:36:16.000000000","message":"after quite some time of review I can say: LGTM!\ntests also pass.\na huge thanks also to the added doc strings.","commit_id":"1796e6d0f26d7899fb05f19d04978271390d44e4"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"67ca50e96823eccf0d51b3b59988b379f4d0f0d5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"2471b0f2_cc8e22bc","updated":"2024-11-05 17:42:50.000000000","message":"let\u0027s go","commit_id":"1796e6d0f26d7899fb05f19d04978271390d44e4"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"ac8ec94a8a8538145403d225235f31577ba9892d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"8e477d08_1591d401","updated":"2024-11-03 11:26:11.000000000","message":"ping","commit_id":"1796e6d0f26d7899fb05f19d04978271390d44e4"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"315a491e66506ddebe14e9c39a600859ff9cb6cb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"94679290_3a8b4594","updated":"2024-11-04 08:06:26.000000000","message":"ping ? Second +2 please ?","commit_id":"1796e6d0f26d7899fb05f19d04978271390d44e4"},{"author":{"_account_id":14200,"name":"Maksim Malchuk","email":"maksim.malchuk@gmail.com","username":"mmalchuk"},"change_message_id":"db37f27c0c3ce233c970e01a88410c97422b2566","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"ce33735f_b80f89f4","updated":"2024-10-09 21:24:47.000000000","message":"thanks for doc strings, lgtm, but I still confused a bit in real cases which can be fixed.","commit_id":"1796e6d0f26d7899fb05f19d04978271390d44e4"},{"author":{"_account_id":14200,"name":"Maksim Malchuk","email":"maksim.malchuk@gmail.com","username":"mmalchuk"},"change_message_id":"d54e0cc189faf560306a1d8905f3f510a1e8bfa1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"2c304c5c_12cdd9b0","in_reply_to":"061eb013_4c916c75","updated":"2024-10-09 22:13:57.000000000","message":"interesting. thanks.","commit_id":"1796e6d0f26d7899fb05f19d04978271390d44e4"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"4a19568b3f1e4da121cb0429c3167db7ee9ac3b7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"061eb013_4c916c75","in_reply_to":"ce33735f_b80f89f4","updated":"2024-10-09 21:36:52.000000000","message":"okay, let\u0027s assume that you provide your own paste.ini or your policy.yaml right ? So in that case there will be entry in config.json to copy ... so after restart that your customized file will be copied and distro file will be overriden.\n\nBut if you change your mind and remove that customized file ...config json will be re-render to not include that file ..so it will be not copied ....so it means you want to use default one ...that\u0027s the thing ...it is already copied from attempt before...\n\nthis will ensure that default will be again in that path","commit_id":"1796e6d0f26d7899fb05f19d04978271390d44e4"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"5a682a02c5010e25473589d61df4cc5351f6ff11","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":22,"id":"0075c352_388e6c5c","updated":"2024-11-06 09:04:10.000000000","message":"recheck bootstrap Temporary failure in name resolution","commit_id":"39a337399c6deca83b98303bc67098792b546095"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"31d2e19623cd5615fe0745118e22d111551dcd72","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":22,"id":"9e6fa8c1_9ddc8f5d","updated":"2024-11-06 08:36:31.000000000","message":"recheck bootstrap Temporary failure in name resolution","commit_id":"39a337399c6deca83b98303bc67098792b546095"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"0253907531c66664c99889a9d2551cc8ddbb5a3e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":22,"id":"5bd3c358_e31d8dd3","updated":"2024-11-06 07:48:15.000000000","message":"recheck ubuntu upgrade time outed","commit_id":"39a337399c6deca83b98303bc67098792b546095"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"15ff09f5e358c2c68e9c4c0128c632abb910d712","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":22,"id":"7478a12e_ba2f1a37","updated":"2024-11-06 03:58:20.000000000","message":"recheck unrelated build error","commit_id":"39a337399c6deca83b98303bc67098792b546095"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"523f6ce04ce704106c4a5798c6ccd05bc78f5097","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"fa29a365_1d7f149b","updated":"2024-11-06 16:22:10.000000000","message":"I vote for Sven because he already gave a +2 earlier, and this patch only differs in the line length of the docstring.","commit_id":"1935d23ba11b1c70dd4079398a14f7e18b63f5a4"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"481cb6d788c7e6120f95bb23214fe86bb9436222","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"fe488974_665b1d38","updated":"2024-11-06 16:23:13.000000000","message":"oops","commit_id":"1935d23ba11b1c70dd4079398a14f7e18b63f5a4"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"b96b6b74d450ca28ffd337be973545c9e3fb387b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"35462111_2b72bdaf","updated":"2024-11-06 16:37:46.000000000","message":"recheck need to get a fresh V+1 for this to be able to proceed","commit_id":"1935d23ba11b1c70dd4079398a14f7e18b63f5a4"},{"author":{"_account_id":14200,"name":"Maksim Malchuk","email":"maksim.malchuk@gmail.com","username":"mmalchuk"},"change_message_id":"ea071f596f2f50d4485d6effa9e1c5748ad60584","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"d9c8bfc7_d4f44271","updated":"2024-11-06 13:00:13.000000000","message":"thanks","commit_id":"1935d23ba11b1c70dd4079398a14f7e18b63f5a4"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"be6f6b18a9d58ac4a8ff6e492412a5f6774a77a5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"c6356781_80b4fd85","updated":"2024-11-06 19:19:50.000000000","message":"Let\u0027s merge","commit_id":"36c12676ff185f524a7f4d2f2f4c70b5d60462b4"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"fe85b1f9c695eb43909178f35bb7bad7479a68be","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"9977c8cb_1718236b","updated":"2024-11-07 06:54:48.000000000","message":"finally - thanks","commit_id":"36c12676ff185f524a7f4d2f2f4c70b5d60462b4"}],"docker/base/set_configs.py":[{"author":{"_account_id":14200,"name":"Maksim Malchuk","email":"maksim.malchuk@gmail.com","username":"mmalchuk"},"change_message_id":"ded94ee548a071ed82784a37c8b55e7962013f42","unresolved":true,"context_lines":[{"line_number":311,"context_line":""},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"def save_defaults(config):"},{"line_number":314,"context_line":"    if \u0027config_files\u0027 in config:"},{"line_number":315,"context_line":"        if not os.path.exists(KOLLA_DEFAULTS):"},{"line_number":316,"context_line":"            os.makedirs(KOLLA_DEFAULTS)"},{"line_number":317,"context_line":"        if not os.path.exists(KOLLA_DEFAULTS_STATE):"}],"source_content_type":"text/x-python","patch_set":2,"id":"44b5ff59_510fd521","line":314,"range":{"start_line":314,"start_character":4,"end_line":314,"end_character":32},"updated":"2024-04-10 22:11:51.000000000","message":"not necessary, we have this check before call this function.","commit_id":"20c7f7983c490f8db202b8837a88c9b7846a9ac6"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"d756aaae076d5d38a7d4745a381ffa6d65c0cd92","unresolved":true,"context_lines":[{"line_number":311,"context_line":""},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"def save_defaults(config):"},{"line_number":314,"context_line":"    if \u0027config_files\u0027 in config:"},{"line_number":315,"context_line":"        if not os.path.exists(KOLLA_DEFAULTS):"},{"line_number":316,"context_line":"            os.makedirs(KOLLA_DEFAULTS)"},{"line_number":317,"context_line":"        if not os.path.exists(KOLLA_DEFAULTS_STATE):"}],"source_content_type":"text/x-python","patch_set":2,"id":"6d3804ca_1fe0f4ca","line":314,"range":{"start_line":314,"start_character":4,"end_line":314,"end_character":32},"in_reply_to":"44b5ff59_510fd521","updated":"2024-04-11 07:27:58.000000000","message":"true, copy paste problem probably","commit_id":"20c7f7983c490f8db202b8837a88c9b7846a9ac6"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"58385f59a8166625d791b909a30ebdb16d43c5f5","unresolved":false,"context_lines":[{"line_number":311,"context_line":""},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"def save_defaults(config):"},{"line_number":314,"context_line":"    if \u0027config_files\u0027 in config:"},{"line_number":315,"context_line":"        if not os.path.exists(KOLLA_DEFAULTS):"},{"line_number":316,"context_line":"            os.makedirs(KOLLA_DEFAULTS)"},{"line_number":317,"context_line":"        if not os.path.exists(KOLLA_DEFAULTS_STATE):"}],"source_content_type":"text/x-python","patch_set":2,"id":"cabc6fa9_75c1135b","line":314,"range":{"start_line":314,"start_character":4,"end_line":314,"end_character":32},"in_reply_to":"6d3804ca_1fe0f4ca","updated":"2024-04-12 14:29:19.000000000","message":"reworked","commit_id":"20c7f7983c490f8db202b8837a88c9b7846a9ac6"},{"author":{"_account_id":14200,"name":"Maksim Malchuk","email":"maksim.malchuk@gmail.com","username":"mmalchuk"},"change_message_id":"ded94ee548a071ed82784a37c8b55e7962013f42","unresolved":true,"context_lines":[{"line_number":345,"context_line":""},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"def handle_defaults():"},{"line_number":348,"context_line":"    with open(KOLLA_DEFAULTS_STATE, \u0027r\u0027) as f:"},{"line_number":349,"context_line":"        state \u003d json.load(f)"},{"line_number":350,"context_line":"    for k, v in state.items():"},{"line_number":351,"context_line":"        if v[\u0027dest\u0027] \u003d\u003d \u0027ABSENT\u0027:"},{"line_number":352,"context_line":"            if os.path.exists(k):"}],"source_content_type":"text/x-python","patch_set":2,"id":"d2225ee3_7c5083f8","line":349,"range":{"start_line":348,"start_character":4,"end_line":349,"end_character":28},"updated":"2024-04-10 22:11:51.000000000","message":"do we really need write file then read it again into the structure? why not use data already we have because handle_defaults used right after save_defaults. there can be global variable or first function can return data to the second one.","commit_id":"20c7f7983c490f8db202b8837a88c9b7846a9ac6"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"58385f59a8166625d791b909a30ebdb16d43c5f5","unresolved":false,"context_lines":[{"line_number":345,"context_line":""},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"def handle_defaults():"},{"line_number":348,"context_line":"    with open(KOLLA_DEFAULTS_STATE, \u0027r\u0027) as f:"},{"line_number":349,"context_line":"        state \u003d json.load(f)"},{"line_number":350,"context_line":"    for k, v in state.items():"},{"line_number":351,"context_line":"        if v[\u0027dest\u0027] \u003d\u003d \u0027ABSENT\u0027:"},{"line_number":352,"context_line":"            if os.path.exists(k):"}],"source_content_type":"text/x-python","patch_set":2,"id":"c877e25d_495da871","line":349,"range":{"start_line":348,"start_character":4,"end_line":349,"end_character":28},"in_reply_to":"712c3095_4ff6df3a","updated":"2024-04-12 14:29:19.000000000","message":"reworked","commit_id":"20c7f7983c490f8db202b8837a88c9b7846a9ac6"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"d756aaae076d5d38a7d4745a381ffa6d65c0cd92","unresolved":true,"context_lines":[{"line_number":345,"context_line":""},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"def handle_defaults():"},{"line_number":348,"context_line":"    with open(KOLLA_DEFAULTS_STATE, \u0027r\u0027) as f:"},{"line_number":349,"context_line":"        state \u003d json.load(f)"},{"line_number":350,"context_line":"    for k, v in state.items():"},{"line_number":351,"context_line":"        if v[\u0027dest\u0027] \u003d\u003d \u0027ABSENT\u0027:"},{"line_number":352,"context_line":"            if os.path.exists(k):"}],"source_content_type":"text/x-python","patch_set":2,"id":"712c3095_4ff6df3a","line":349,"range":{"start_line":348,"start_character":4,"end_line":349,"end_character":28},"in_reply_to":"d2225ee3_7c5083f8","updated":"2024-04-11 07:27:58.000000000","message":"Whole script running 9ms, is really helpful to hide something to GLOBAL var somewhere upside ? or let\u0027s say make readibility of code worse ? \n\nIt\u0027s json.load ... c\u0027mon","commit_id":"20c7f7983c490f8db202b8837a88c9b7846a9ac6"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"91219819a5ff85fa2f8dfd6e7cf4990ab3bd0d7f","unresolved":true,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"# TODO(rhallisey): add docstring."},{"line_number":28,"context_line":"logging.basicConfig("},{"line_number":29,"context_line":"    format\u003d\u0027%(asctime)s.%(msecs)03d %(levelname)s %(message)s\u0027,"},{"line_number":30,"context_line":"    level\u003dlogging.INFO, datefmt\u003d\u0027%Y-%m-%d %H:%M:%S\u0027"},{"line_number":31,"context_line":")"},{"line_number":32,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":7,"id":"ad501c71_dabf4f04","line":29,"updated":"2024-04-17 12:43:05.000000000","message":"this seems to not be related with the patchset - different patch?","commit_id":"1b79d90efa647d5beffbfc44984427c226f96e79"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"266c1a34ac9060f5711acfd847ac1f41ccd59a5f","unresolved":true,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"# TODO(rhallisey): add docstring."},{"line_number":28,"context_line":"logging.basicConfig("},{"line_number":29,"context_line":"    format\u003d\u0027%(asctime)s.%(msecs)03d %(levelname)s %(message)s\u0027,"},{"line_number":30,"context_line":"    level\u003dlogging.INFO, datefmt\u003d\u0027%Y-%m-%d %H:%M:%S\u0027"},{"line_number":31,"context_line":")"},{"line_number":32,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":7,"id":"821cb6ee_73d7819f","line":29,"in_reply_to":"1c2638ec_8e3ed8ee","updated":"2024-09-18 10:38:40.000000000","message":"@mnasiadka@gmail.com So ? What do you mean ? Should I move to new review and close this comment ? Or just close as it\u0027s tiny change and useful ?","commit_id":"1b79d90efa647d5beffbfc44984427c226f96e79"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"3c0eec8c6d9a70f26ededdb005c860f00ae650ce","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"# TODO(rhallisey): add docstring."},{"line_number":28,"context_line":"logging.basicConfig("},{"line_number":29,"context_line":"    format\u003d\u0027%(asctime)s.%(msecs)03d %(levelname)s %(message)s\u0027,"},{"line_number":30,"context_line":"    level\u003dlogging.INFO, datefmt\u003d\u0027%Y-%m-%d %H:%M:%S\u0027"},{"line_number":31,"context_line":")"},{"line_number":32,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":7,"id":"ce161456_88f222ed","line":29,"in_reply_to":"56040b14_3960887e","updated":"2024-09-21 22:33:53.000000000","message":"Moved to review https://review.opendev.org/c/openstack/kolla/+/930152","commit_id":"1b79d90efa647d5beffbfc44984427c226f96e79"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"24760de8718ae82a8e42d7f4a20fe5c77374d2eb","unresolved":true,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"# TODO(rhallisey): add docstring."},{"line_number":28,"context_line":"logging.basicConfig("},{"line_number":29,"context_line":"    format\u003d\u0027%(asctime)s.%(msecs)03d %(levelname)s %(message)s\u0027,"},{"line_number":30,"context_line":"    level\u003dlogging.INFO, datefmt\u003d\u0027%Y-%m-%d %H:%M:%S\u0027"},{"line_number":31,"context_line":")"},{"line_number":32,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":7,"id":"ae05c169_184cc99b","line":29,"in_reply_to":"821cb6ee_73d7819f","updated":"2024-09-18 13:36:43.000000000","message":"Well, if we find the format is for some reason breaking something, I\u0027d prefer to have it as a separate patch - so new patch please.","commit_id":"1b79d90efa647d5beffbfc44984427c226f96e79"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"97292f154767771ddaa7ab072b8db5c805f91714","unresolved":true,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"# TODO(rhallisey): add docstring."},{"line_number":28,"context_line":"logging.basicConfig("},{"line_number":29,"context_line":"    format\u003d\u0027%(asctime)s.%(msecs)03d %(levelname)s %(message)s\u0027,"},{"line_number":30,"context_line":"    level\u003dlogging.INFO, datefmt\u003d\u0027%Y-%m-%d %H:%M:%S\u0027"},{"line_number":31,"context_line":")"},{"line_number":32,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1c2638ec_8e3ed8ee","line":29,"in_reply_to":"ad501c71_dabf4f04","updated":"2024-04-30 10:40:15.000000000","message":"If you want to move one line into another patch called \"change the format of docker container logging\" ... i am ok with it ...but i think it is unnecessary\n\nPlease, let me know and i will do it ...","commit_id":"1b79d90efa647d5beffbfc44984427c226f96e79"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"7fb88d408f41caf36efb4c9a721a33cbc09eca6f","unresolved":true,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"# TODO(rhallisey): add docstring."},{"line_number":28,"context_line":"logging.basicConfig("},{"line_number":29,"context_line":"    format\u003d\u0027%(asctime)s.%(msecs)03d %(levelname)s %(message)s\u0027,"},{"line_number":30,"context_line":"    level\u003dlogging.INFO, datefmt\u003d\u0027%Y-%m-%d %H:%M:%S\u0027"},{"line_number":31,"context_line":")"},{"line_number":32,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":7,"id":"56040b14_3960887e","line":29,"in_reply_to":"ae05c169_184cc99b","updated":"2024-09-18 13:41:37.000000000","message":"Okay, no problem.","commit_id":"1b79d90efa647d5beffbfc44984427c226f96e79"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"d2536d34e5ce55996b6411b593f5c2a0f8eede56","unresolved":true,"context_lines":[{"line_number":405,"context_line":"        json.dump(state, f, indent\u003d4)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"def handle_defaults(config):"},{"line_number":409,"context_line":"    state \u003d get_defaults_state()"},{"line_number":410,"context_line":"    for k, v in state.items():"},{"line_number":411,"context_line":"        if v[\u0027dest\u0027] is None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"eb72c84c_53360ad3","line":408,"range":{"start_line":408,"start_character":0,"end_line":408,"end_character":2},"updated":"2024-04-17 12:12:27.000000000","message":"maybe I\u0027m an idiot, but I would like to have a docstring here that explains the algorithm in plain text, as I think it\u0027s complicated enough to warrant that.\nI\u0027m not sure I would miss simple errors during review.","commit_id":"cb233d1ed0f38043978322ec697f07f18b73611c"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"43518e30790ec7a09d589bf90f9f4bb0d800007d","unresolved":true,"context_lines":[{"line_number":405,"context_line":"        json.dump(state, f, indent\u003d4)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"def handle_defaults(config):"},{"line_number":409,"context_line":"    state \u003d get_defaults_state()"},{"line_number":410,"context_line":"    for k, v in state.items():"},{"line_number":411,"context_line":"        if v[\u0027dest\u0027] is None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ad1a1936_ff6bf976","line":408,"range":{"start_line":408,"start_character":0,"end_line":408,"end_character":2},"in_reply_to":"96e8a457_4d43c2cd","updated":"2024-07-10 10:40:04.000000000","message":"+1 to this. I would even go further and ask for some tests that verify that each of the code paths does what is expected","commit_id":"cb233d1ed0f38043978322ec697f07f18b73611c"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"5911822117b07b30de6a3c06845477868d4b1552","unresolved":false,"context_lines":[{"line_number":405,"context_line":"        json.dump(state, f, indent\u003d4)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"def handle_defaults(config):"},{"line_number":409,"context_line":"    state \u003d get_defaults_state()"},{"line_number":410,"context_line":"    for k, v in state.items():"},{"line_number":411,"context_line":"        if v[\u0027dest\u0027] is None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ec4d9026_bcafcfb0","line":408,"range":{"start_line":408,"start_character":0,"end_line":408,"end_character":2},"in_reply_to":"ad1a1936_ff6bf976","updated":"2024-09-17 10:28:53.000000000","message":"Done","commit_id":"cb233d1ed0f38043978322ec697f07f18b73611c"},{"author":{"_account_id":14200,"name":"Maksim Malchuk","email":"maksim.malchuk@gmail.com","username":"mmalchuk"},"change_message_id":"165b6efc1dce07c2b5389bf0f4d2a85fac2b8bad","unresolved":true,"context_lines":[{"line_number":405,"context_line":"        json.dump(state, f, indent\u003d4)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"def handle_defaults(config):"},{"line_number":409,"context_line":"    state \u003d get_defaults_state()"},{"line_number":410,"context_line":"    for k, v in state.items():"},{"line_number":411,"context_line":"        if v[\u0027dest\u0027] is None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"fc5c44de_6b5839e0","line":408,"range":{"start_line":408,"start_character":0,"end_line":408,"end_character":2},"in_reply_to":"eb72c84c_53360ad3","updated":"2024-04-17 12:25:09.000000000","message":"agree with Sven. this was requested in the 2nd patch set. algorithm or logic is not obvious.","commit_id":"cb233d1ed0f38043978322ec697f07f18b73611c"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"e42eb7347ec961ff33eef248d56366b82ed64125","unresolved":true,"context_lines":[{"line_number":405,"context_line":"        json.dump(state, f, indent\u003d4)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"def handle_defaults(config):"},{"line_number":409,"context_line":"    state \u003d get_defaults_state()"},{"line_number":410,"context_line":"    for k, v in state.items():"},{"line_number":411,"context_line":"        if v[\u0027dest\u0027] is None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"96e8a457_4d43c2cd","line":408,"range":{"start_line":408,"start_character":0,"end_line":408,"end_character":2},"in_reply_to":"fc5c44de_6b5839e0","updated":"2024-04-17 12:43:53.000000000","message":"Agree 100%, it\u0027s hard to understand what is the end goal here, not even from the commit message.","commit_id":"cb233d1ed0f38043978322ec697f07f18b73611c"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"d2536d34e5ce55996b6411b593f5c2a0f8eede56","unresolved":true,"context_lines":[{"line_number":407,"context_line":""},{"line_number":408,"context_line":"def handle_defaults(config):"},{"line_number":409,"context_line":"    state \u003d get_defaults_state()"},{"line_number":410,"context_line":"    for k, v in state.items():"},{"line_number":411,"context_line":"        if v[\u0027dest\u0027] is None:"},{"line_number":412,"context_line":"            if os.path.exists(k):"},{"line_number":413,"context_line":"                if os.path.isfile(k):"},{"line_number":414,"context_line":"                    os.remove(k)"},{"line_number":415,"context_line":"                else:"},{"line_number":416,"context_line":"                    shutil.rmtree(k)"},{"line_number":417,"context_line":"        else:"},{"line_number":418,"context_line":"            v[\u0027source\u0027], v[\u0027dest\u0027] \u003d v[\u0027dest\u0027], v[\u0027source\u0027]"},{"line_number":419,"context_line":"            config_file \u003d ConfigFile(**v)"}],"source_content_type":"text/x-python","patch_set":8,"id":"465150aa_81a1e52e","line":416,"range":{"start_line":410,"start_character":0,"end_line":416,"end_character":36},"updated":"2024-04-17 12:12:27.000000000","message":"Why don\u0027t we always use shutil.rmtree here? afaik it also can remove files, no?\n\nif we want to be extra secure we could detect if our platform is suspectible to symlink attacks via: https://docs.python.org/3/library/shutil.html#shutil.rmtree.avoids_symlink_attacks before using rmtree.","commit_id":"cb233d1ed0f38043978322ec697f07f18b73611c"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"418cd3a813687ddbd0dc72b9dcc9afc51af35b32","unresolved":false,"context_lines":[{"line_number":407,"context_line":""},{"line_number":408,"context_line":"def handle_defaults(config):"},{"line_number":409,"context_line":"    state \u003d get_defaults_state()"},{"line_number":410,"context_line":"    for k, v in state.items():"},{"line_number":411,"context_line":"        if v[\u0027dest\u0027] is None:"},{"line_number":412,"context_line":"            if os.path.exists(k):"},{"line_number":413,"context_line":"                if os.path.isfile(k):"},{"line_number":414,"context_line":"                    os.remove(k)"},{"line_number":415,"context_line":"                else:"},{"line_number":416,"context_line":"                    shutil.rmtree(k)"},{"line_number":417,"context_line":"        else:"},{"line_number":418,"context_line":"            v[\u0027source\u0027], v[\u0027dest\u0027] \u003d v[\u0027dest\u0027], v[\u0027source\u0027]"},{"line_number":419,"context_line":"            config_file \u003d ConfigFile(**v)"}],"source_content_type":"text/x-python","patch_set":8,"id":"d353dcd5_679dc1d0","line":416,"range":{"start_line":410,"start_character":0,"end_line":416,"end_character":36},"in_reply_to":"465150aa_81a1e52e","updated":"2024-04-30 10:25:47.000000000","message":"1, shutil.rmtree can\u0027t \n\ncheck below : \n\nmichalarbet@pixla:~/ultimum/git/ultimum/kolla-config$ touch /tmp/sven_test\nmichalarbet@pixla:~/ultimum/git/ultimum/kolla-config$ python3\nPython 3.11.6 (main, Oct  8 2023, 05:06:43) [GCC 13.2.0] on linux\nType \"help\", \"copyright\", \"credits\" or \"license\" for more information.\n\u003e\u003e\u003e import shutil\n\u003e\u003e\u003e shutil.rmtree(\u0027/tmp/sven_test\u0027)\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n  File \"/usr/lib/python3.11/shutil.py\", line 732, in rmtree\n    _rmtree_safe_fd(fd, path, onerror)\n  File \"/usr/lib/python3.11/shutil.py\", line 635, in _rmtree_safe_fd\n    onerror(os.scandir, path, sys.exc_info())\n  File \"/usr/lib/python3.11/shutil.py\", line 631, in _rmtree_safe_fd\n    with os.scandir(topfd) as scandir_it:\n         ^^^^^^^^^^^^^^^^^\nNotADirectoryError: [Errno 20] Not a directory: \u0027/tmp/sven_test\u0027\n\u003e\u003e\u003e import os\n\u003e\u003e\u003e os.remove(\u0027/tmp/sven_test\u0027)\n\nAre you sure about that statement when you wrote it ? \n\n2, shutil.rmtree function in Python has the ability to avoid symlink attacks by default, starting from Python 3.8 with the shutil.avoids_symlink_attacks parameter. This parameter, when set to True, prevents rmtree from following symbolic links during directory tree removal, thereby mitigating symlink attacks.\n\n\nmichalarbet@pixla:~/ultimum/git/ultimum/kolla-ansible$ python3\nPython 3.11.6 (main, Oct  8 2023, 05:06:43) [GCC 13.2.0] on linux\nType \"help\", \"copyright\", \"credits\" or \"license\" for more information.\n\u003e\u003e\u003e import os\n\u003e\u003e\u003e import shutil\n\u003e\u003e\u003e os.path.isfile(\u0027/tmp/sven_link\u0027)\nFalse\n\u003e\u003e\u003e os.path.isdir(\u0027/tmp/sven_link\u0027)\nTrue\n\u003e\u003e\u003e shutil.rmtree(\u0027/tmp/sven_link\u0027)\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n  File \"/usr/lib/python3.11/shutil.py\", line 744, in rmtree\n    onerror(os.path.islink, path, sys.exc_info())\n  File \"/usr/lib/python3.11/shutil.py\", line 742, in rmtree\n    raise OSError(\"Cannot call rmtree on a symbolic link\")\nOSError: Cannot call rmtree on a symbolic link\n\nSo where is the problem ? Handle exception ? Because there is also rmtree used in function _delete_path (not my code) and used in same way below ...\n\n    def _delete_path(self, path):\n        if not os.path.lexists(path):\n            return\n        LOG.info(\u0027Deleting %s\u0027, path)\n        if os.path.isdir(path):\n            shutil.rmtree(path)\n        else:\n            os.remove(path)\n            \nMarking as resolved ... reopen if you don\u0027t agree.","commit_id":"cb233d1ed0f38043978322ec697f07f18b73611c"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"d2536d34e5ce55996b6411b593f5c2a0f8eede56","unresolved":true,"context_lines":[{"line_number":425,"context_line":"                continue"},{"line_number":426,"context_line":"            src \u003d data[\u0027source\u0027]"},{"line_number":427,"context_line":"            dst \u003d data[\u0027dest\u0027]"},{"line_number":428,"context_line":"            dflt \u003d KOLLA_DEFAULTS + dst"},{"line_number":429,"context_line":"            if os.path.exists(src):"},{"line_number":430,"context_line":"                copy \u003d {\u0027source\u0027: dst, \u0027preserve_properties\u0027: True}"},{"line_number":431,"context_line":"                if os.path.exists(dst):"}],"source_content_type":"text/x-python","patch_set":8,"id":"25a792d2_c6ca0798","line":428,"range":{"start_line":428,"start_character":12,"end_line":428,"end_character":17},"updated":"2024-04-17 12:12:27.000000000","message":"just spell it out? \"default\"?","commit_id":"cb233d1ed0f38043978322ec697f07f18b73611c"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"3e2c382fb3a8581fc3a19f3edf5da049cc26e679","unresolved":false,"context_lines":[{"line_number":425,"context_line":"                continue"},{"line_number":426,"context_line":"            src \u003d data[\u0027source\u0027]"},{"line_number":427,"context_line":"            dst \u003d data[\u0027dest\u0027]"},{"line_number":428,"context_line":"            dflt \u003d KOLLA_DEFAULTS + dst"},{"line_number":429,"context_line":"            if os.path.exists(src):"},{"line_number":430,"context_line":"                copy \u003d {\u0027source\u0027: dst, \u0027preserve_properties\u0027: True}"},{"line_number":431,"context_line":"                if os.path.exists(dst):"}],"source_content_type":"text/x-python","patch_set":8,"id":"495f8b30_db0c607a","line":428,"range":{"start_line":428,"start_character":12,"end_line":428,"end_character":17},"in_reply_to":"25a792d2_c6ca0798","updated":"2024-04-30 15:17:24.000000000","message":"Done","commit_id":"cb233d1ed0f38043978322ec697f07f18b73611c"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"c30d1093de69cb47e2e711caee9ca9f4c0d65bd6","unresolved":true,"context_lines":[{"line_number":404,"context_line":""},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"def handle_defaults(config):"},{"line_number":407,"context_line":"    state \u003d get_defaults_state()"},{"line_number":408,"context_line":"    for k, v in state.items():"},{"line_number":409,"context_line":"        if v[\u0027dest\u0027] is None:"},{"line_number":410,"context_line":"            if os.path.exists(k):"},{"line_number":411,"context_line":"                if os.path.isfile(k):"}],"source_content_type":"text/x-python","patch_set":9,"id":"d295f306_d850714e","line":408,"range":{"start_line":407,"start_character":0,"end_line":408,"end_character":30},"updated":"2024-04-30 12:56:42.000000000","message":"imho you need to check here if state actually contains anything because `get_defaults_state()` might return an empty dict.\n\nor maybe just return an error in the get_defaults_state() function if we never expect defaults to not be available, not sure.","commit_id":"a84ce7436e571dc7a059f41d869d01f0e2d6a0b3"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"3e2c382fb3a8581fc3a19f3edf5da049cc26e679","unresolved":true,"context_lines":[{"line_number":404,"context_line":""},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"def handle_defaults(config):"},{"line_number":407,"context_line":"    state \u003d get_defaults_state()"},{"line_number":408,"context_line":"    for k, v in state.items():"},{"line_number":409,"context_line":"        if v[\u0027dest\u0027] is None:"},{"line_number":410,"context_line":"            if os.path.exists(k):"},{"line_number":411,"context_line":"                if os.path.isfile(k):"}],"source_content_type":"text/x-python","patch_set":9,"id":"d455d53d_b9379ca1","line":408,"range":{"start_line":407,"start_character":0,"end_line":408,"end_character":30},"in_reply_to":"d295f306_d850714e","updated":"2024-04-30 15:17:24.000000000","message":"Yeah, state can be empty and this code count with it. If it is empty, no defaults will be restored or files removed. Iterate through empty dict is completly valid.\n\nPython 3.11.6 (main, Oct  8 2023, 05:06:43) [GCC 13.2.0] on linux\nType \"help\", \"copyright\", \"credits\" or \"license\" for more information.\n\u003e\u003e\u003e state \u003d {}\n\u003e\u003e\u003e for k,v in state.items():\n...     print(f\"{k}:{v}\")\n... \n\n\nBut on the end of the function state is setted by set_defaults_state.\n\nI don\u0027t see any problem ... Where you can see a problem ? I think i don\u0027t understand.","commit_id":"a84ce7436e571dc7a059f41d869d01f0e2d6a0b3"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"c4463306f48d0b4847a2bf140aea0ef66657613a","unresolved":false,"context_lines":[{"line_number":404,"context_line":""},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"def handle_defaults(config):"},{"line_number":407,"context_line":"    state \u003d get_defaults_state()"},{"line_number":408,"context_line":"    for k, v in state.items():"},{"line_number":409,"context_line":"        if v[\u0027dest\u0027] is None:"},{"line_number":410,"context_line":"            if os.path.exists(k):"},{"line_number":411,"context_line":"                if os.path.isfile(k):"}],"source_content_type":"text/x-python","patch_set":9,"id":"543e7257_2e0fc3b7","line":408,"range":{"start_line":407,"start_character":0,"end_line":408,"end_character":30},"in_reply_to":"d455d53d_b9379ca1","updated":"2024-05-27 12:55:10.000000000","message":"not really a \"problem\", more a question about style if we want to rely on the implicit python mechanic which handles iterating over empty dicts, which probably won\u0027t change in the future.\n\nit\u0027s more a matter of \"explicit is better than implicit\", which I happen to like as a coding guideline.\n\nSo I just \"ack\" this, as I\u0027m not aware that we have any coding guidelines around this topic and it\u0027s arguably a matter of taste.","commit_id":"a84ce7436e571dc7a059f41d869d01f0e2d6a0b3"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"32daf5b8852dff27fb9cf8c4446841313854614d","unresolved":true,"context_lines":[{"line_number":413,"context_line":"                else:"},{"line_number":414,"context_line":"                    shutil.rmtree(k)"},{"line_number":415,"context_line":"        else:"},{"line_number":416,"context_line":"            v[\u0027source\u0027], v[\u0027dest\u0027] \u003d v[\u0027dest\u0027], v[\u0027source\u0027]"},{"line_number":417,"context_line":"            config_file \u003d ConfigFile(**v)"},{"line_number":418,"context_line":"            config_file.copy()"},{"line_number":419,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"19929fff_3148a4ad","line":416,"range":{"start_line":416,"start_character":0,"end_line":416,"end_character":2},"updated":"2024-04-30 12:38:20.000000000","message":"I honestly don\u0027t currently understand this. why do we swap source and dest values here? maybe it\u0027s just my brain not working, I guess there will be a good reason 😄","commit_id":"a84ce7436e571dc7a059f41d869d01f0e2d6a0b3"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"aeaf0a8225df1a57176562ea918f9bcbe128592d","unresolved":false,"context_lines":[{"line_number":413,"context_line":"                else:"},{"line_number":414,"context_line":"                    shutil.rmtree(k)"},{"line_number":415,"context_line":"        else:"},{"line_number":416,"context_line":"            v[\u0027source\u0027], v[\u0027dest\u0027] \u003d v[\u0027dest\u0027], v[\u0027source\u0027]"},{"line_number":417,"context_line":"            config_file \u003d ConfigFile(**v)"},{"line_number":418,"context_line":"            config_file.copy()"},{"line_number":419,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"c051091f_27293c7d","line":416,"range":{"start_line":416,"start_character":0,"end_line":416,"end_character":2},"in_reply_to":"19929fff_3148a4ad","updated":"2024-04-30 13:22:38.000000000","message":"Okay, no problem to explain of course : \n\nInside container : \n(glance-api)[root@controller0 /]# echo \u0027original kevko created\u0027 \u003e /tmp/kevko\n\nCreated new file and add entry to config.json for glance-api service\n\nroot@controller0:~# docker exec -itu root glance_api bash \n(glance-api)[root@controller0 /]# find /etc/kolla/defaults/\n/etc/kolla/defaults/\n/etc/kolla/defaults/etc\n/etc/kolla/defaults/etc/glance\n/etc/kolla/defaults/etc/glance/glance-api.conf\n/etc/kolla/defaults/etc/ceph\n/etc/kolla/defaults/etc/ceph/ceph.conf\n/etc/kolla/defaults/etc/ceph/ceph.client.glance.keyring\n/etc/kolla/defaults/state\n/etc/kolla/defaults/tmp\n/etc/kolla/defaults/tmp/kevko\n\n\nYou can see that tmp/kevko is backuped now, so let\u0027s check what was the state ...\n\n(glance-api)[root@controller0 /]# cat /etc/kolla/defaults/state \n{\n    \"/etc/glance/glance-api.conf\": {\n        \"source\": \"/etc/glance/glance-api.conf\",\n        \"preserve_properties\": true,\n        \"dest\": \"/etc/kolla/defaults/etc/glance/glance-api.conf\"\n    },\n    \"/tmp/kevko\": {\n        \"source\": \"/tmp/kevko\",\n        \"preserve_properties\": true,\n        \"dest\": \"/etc/kolla/defaults/tmp/kevko\"\n    },\n    \"/etc/ceph\": {\n        \"source\": \"/etc/ceph\",\n        \"preserve_properties\": true,\n        \"dest\": \"/etc/kolla/defaults/etc/ceph\"\n    }\n}\n\nSo, you can see the structure is dict where key is the dest path from config.json ... but source and destination is different as this is used by my handle function and others ...\n\nSo if the code is backuping ...it\u0027s copying src -\u003e dst \n\nSo for /tmp/kevko it\u0027s    /tmp/kevko -\u003e /etc/kolla/defaults/tmp/kevko\n\nBut there is also reverse functionality when container starting and restoring defaults ...so we are copying dest -\u003e src  soooooo\n\n/etc/kolla/defaults/tmp/kevko -\u003e /tmp/kevko\n\nAfter this if config.json still have an entry for copying kevko file ..it\u0027s just copied ..no problem .... but if not ... default is there as it is restored ..or deleted if the state of that file was absent on the beginning\n\nOn the end let\u0027s check the content \n\n}(glance-api)[root@controller0 /]# cat /tmp/kevko \ninjected from host into container\n\n\nOkay, let\u0027s remove entry from config.json and check again \n\nroot@controller0:~# cp -av /etc/kolla/glance-api/config.json.backup /etc/kolla/glance-api/config.json \n\u0027/etc/kolla/glance-api/config.json.backup\u0027 -\u003e \u0027/etc/kolla/glance-api/config.json\u0027\nroot@controller0:~# docker restart glance_api\nglance_api\nroot@controller0:~# docker exec -itu root glance_api bash \n(glance-api)[root@controller0 /]# cat /tmp/kevko \noriginal kevko created\n\n\n\nWorking ...\n\n\nTLDR : \n\nsrc \u003c-\u003e dst is switched when restoring defaults to their paths","commit_id":"a84ce7436e571dc7a059f41d869d01f0e2d6a0b3"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"cc7e5000c9dcca0cffd61ef09705ecd5db45f0c7","unresolved":false,"context_lines":[{"line_number":413,"context_line":"                else:"},{"line_number":414,"context_line":"                    shutil.rmtree(k)"},{"line_number":415,"context_line":"        else:"},{"line_number":416,"context_line":"            v[\u0027source\u0027], v[\u0027dest\u0027] \u003d v[\u0027dest\u0027], v[\u0027source\u0027]"},{"line_number":417,"context_line":"            config_file \u003d ConfigFile(**v)"},{"line_number":418,"context_line":"            config_file.copy()"},{"line_number":419,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"dd700452_4a9dfe46","line":416,"range":{"start_line":416,"start_character":0,"end_line":416,"end_character":2},"in_reply_to":"c051091f_27293c7d","updated":"2024-04-30 13:34:09.000000000","message":"thanks for the detailed explanation, makes sense now. :)","commit_id":"a84ce7436e571dc7a059f41d869d01f0e2d6a0b3"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"c30d1093de69cb47e2e711caee9ca9f4c0d65bd6","unresolved":true,"context_lines":[{"line_number":424,"context_line":"            src \u003d data[\u0027source\u0027]"},{"line_number":425,"context_line":"            dst \u003d data[\u0027dest\u0027]"},{"line_number":426,"context_line":"            dflt \u003d KOLLA_DEFAULTS + dst"},{"line_number":427,"context_line":"            if os.path.exists(src):"},{"line_number":428,"context_line":"                copy \u003d {\u0027source\u0027: dst, \u0027preserve_properties\u0027: True}"},{"line_number":429,"context_line":"                if os.path.exists(dst):"},{"line_number":430,"context_line":"                    copy[\u0027dest\u0027] \u003d dflt"}],"source_content_type":"text/x-python","patch_set":9,"id":"cc8645c9_2a02aa04","line":427,"range":{"start_line":427,"start_character":0,"end_line":427,"end_character":2},"updated":"2024-04-30 12:56:42.000000000","message":"not sure this is needed here, as said in IRC, I\u0027m not terribly familiar with this code part, but this ends up just calling ConfigFile.copy() function which already checks that the source exists.","commit_id":"a84ce7436e571dc7a059f41d869d01f0e2d6a0b3"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"3e2c382fb3a8581fc3a19f3edf5da049cc26e679","unresolved":false,"context_lines":[{"line_number":424,"context_line":"            src \u003d data[\u0027source\u0027]"},{"line_number":425,"context_line":"            dst \u003d data[\u0027dest\u0027]"},{"line_number":426,"context_line":"            dflt \u003d KOLLA_DEFAULTS + dst"},{"line_number":427,"context_line":"            if os.path.exists(src):"},{"line_number":428,"context_line":"                copy \u003d {\u0027source\u0027: dst, \u0027preserve_properties\u0027: True}"},{"line_number":429,"context_line":"                if os.path.exists(dst):"},{"line_number":430,"context_line":"                    copy[\u0027dest\u0027] \u003d dflt"}],"source_content_type":"text/x-python","patch_set":9,"id":"d90e821b_64acfe6c","line":427,"range":{"start_line":427,"start_character":0,"end_line":427,"end_character":2},"in_reply_to":"cc8645c9_2a02aa04","updated":"2024-04-30 15:17:24.000000000","message":"That might be true, but from the perspective of the concept in this function, the logic is beautifully visible when it\u0027s written like this. That means this first \u0027if\u0027 and then another \u0027if\u0027 checking destination, and \u0027else\u0027 associated with it... \n\nIt makes it more visible and gives more sense at first glance.","commit_id":"a84ce7436e571dc7a059f41d869d01f0e2d6a0b3"}]}