)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"9bd63ccf2f8c03e31bbca075b67193935883dc01","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"53856bc2_17889e9e","updated":"2021-11-01 15:16:01.000000000","message":"This LGTM, but there is a merge conflict :-(","commit_id":"dd79642e511b615c68170d3fe8771be7f975af8e"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"bb8f1f2c59fc51d77625b39745bfd5ace30444f1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"8a7e019a_c0fcba70","updated":"2021-11-02 14:01:41.000000000","message":"Thanks Steve. I\u0027ve got some suggestions.","commit_id":"71f0b5eeaad68c8d2292a66c5c5f86a725e9c2e8"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"6247a08a21ea613ab4e651bb11059d7f238002c5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"e91b462f_4f34648d","updated":"2021-12-02 21:15:51.000000000","message":"Thanks!","commit_id":"786f9bb0c9feebd8d3261fe353902ae62010381c"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"e450cdaa2fc033afd48011418b6023f80a1bdf33","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"be5489c3_de122427","updated":"2021-12-02 00:55:39.000000000","message":"recheck","commit_id":"786f9bb0c9feebd8d3261fe353902ae62010381c"}],"ironic/common/pxe_utils.py":[{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ef1881629943a70df25c2fbfc04761ad213f64e8","unresolved":true,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"def ensure_tree(path):"},{"line_number":76,"context_line":"    mode \u003d CONF.pxe.dir_permission or 0o777"},{"line_number":77,"context_line":"    os.makedirs(path, mode, exist_ok\u003dTrue)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"64234054_e8d64206","line":76,"range":{"start_line":76,"start_character":35,"end_line":76,"end_character":43},"updated":"2021-09-15 23:13:55.000000000","message":"world writable would catch us in some hot water as we would be functionally disregarding umask all the time... but a file_permission might make sense, or a default permission maybe?","commit_id":"6d7c4f03b7209daef3777bab3c4a50c57635bd20"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"aa4c61622d93e7e52457f975a41964aa33920c94","unresolved":true,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"def ensure_tree(path):"},{"line_number":76,"context_line":"    mode \u003d CONF.pxe.dir_permission or 0o777"},{"line_number":77,"context_line":"    os.makedirs(path, mode, exist_ok\u003dTrue)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"377068b1_2af74552","line":76,"range":{"start_line":76,"start_character":35,"end_line":76,"end_character":43},"in_reply_to":"64234054_e8d64206","updated":"2021-09-16 00:10:10.000000000","message":"Thats what I thought, but both fileutils.ensure_tree and os.makedirs also default to 0o777. So I tried that locally:\n\n  \u003e\u003e\u003e import os\n  \u003e\u003e\u003e os.makedirs(\u0027bar\u0027, mode\u003d0o777)\n  \u003e\u003e\u003e \n  [root@sbaker-t590 foo]# ls -l\n  total 8\n  drwxr-xr-x. 2 root root 4096 Sep 16 10:44 bar\n  drwxr-xr-x. 2 root root 4096 Sep 16 10:36 foo\n  [root@sbaker-t590 foo]# umask\n  0022\n\nAlso the docs[1] say of the mode argument: \"Where it is used, the current umask value is first masked out.\"\n\nSo I think 0o777 is appropriate.\n\n[1] https://docs.python.org/3/library/os.html#os.mkdir","commit_id":"6d7c4f03b7209daef3777bab3c4a50c57635bd20"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"865fb1e2fc90767fadde37b9be9d181cd0e2ebc4","unresolved":true,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":"def ensure_tree(path):"},{"line_number":77,"context_line":"    mode \u003d CONF.pxe.dir_permission or 0o777"},{"line_number":78,"context_line":"    os.makedirs(path, mode, exist_ok\u003dTrue)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"def _ensure_config_dirs_exist(task, ipxe_enabled\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":3,"id":"f9606c8f_fddb1f39","line":78,"updated":"2021-11-01 15:26:26.000000000","message":"nit s/mode/mode\u003dmode/\n\nmight be useful to add a comment that the current umask value is first masked out of \u0027mode\u0027.\n\nOH. Any idea which platforms this might apply to, wrt \u0027mode\u0027? \"On some platforms, they are ignored and you should call chmod() explicitly to set them\". (from https://docs.python.org/3/library/os.html#os.mkdir).","commit_id":"dd79642e511b615c68170d3fe8771be7f975af8e"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"cf6b6c1c20b97cf20824bd46c2a5ce383b6b1cae","unresolved":true,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":"def ensure_tree(path):"},{"line_number":77,"context_line":"    mode \u003d CONF.pxe.dir_permission or 0o777"},{"line_number":78,"context_line":"    os.makedirs(path, mode, exist_ok\u003dTrue)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"def _ensure_config_dirs_exist(task, ipxe_enabled\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":3,"id":"257017a3_46ed7f45","line":78,"in_reply_to":"f9606c8f_fddb1f39","updated":"2021-11-02 04:33:38.000000000","message":"\u003e nit s/mode/mode\u003dmode/\nDone\n\n\u003e might be useful to add a comment that the current umask value is first masked out of \u0027mode\u0027.\nAgreed, I\u0027ve done it in the pxe conf help. Its a non-obvious python quirk\n\n\u003e OH. Any idea which platforms this might apply to, wrt \u0027mode\u0027?\nIt is quite ambiguous, but I read the \"On some platforms \" sentence as referring to \"bits other than the last 9\" in the sentence before. Either way I would hazard that \"some platforms\" refers to platforms we don\u0027t care about ;)","commit_id":"dd79642e511b615c68170d3fe8771be7f975af8e"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"c90e1bdda4e2a81ec523d0d7cec9ae045041acbe","unresolved":true,"context_lines":[{"line_number":74,"context_line":""},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"def ensure_tree(path):"},{"line_number":77,"context_line":"    mode \u003d CONF.pxe.dir_permission or 0o777"},{"line_number":78,"context_line":"    os.makedirs(path, mode\u003dmode, exist_ok\u003dTrue)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"faa52ab4_98c3b763","line":77,"range":{"start_line":77,"start_character":38,"end_line":77,"end_character":43},"updated":"2021-12-01 21:22:21.000000000","message":"I\u0027ll change this to 0o755 for consistency","commit_id":"71f0b5eeaad68c8d2292a66c5c5f86a725e9c2e8"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"bb8f1f2c59fc51d77625b39745bfd5ace30444f1","unresolved":true,"context_lines":[{"line_number":1296,"context_line":"                try:"},{"line_number":1297,"context_line":"                    os.makedirs("},{"line_number":1298,"context_line":"                        os.path.join(base_path, head),"},{"line_number":1299,"context_line":"                        mode\u003dCONF.pxe.dir_permission or 0o755, exist_ok\u003dTrue)"},{"line_number":1300,"context_line":"                except OSError as e:"},{"line_number":1301,"context_line":"                    msg \u003d (\u0027Failed to create supplied directories in \u0027"},{"line_number":1302,"context_line":"                           \u0027asset copy paths. Error: %s\u0027) % e"}],"source_content_type":"text/x-python","patch_set":4,"id":"c6038bbd_c717fd2f","line":1299,"updated":"2021-11-02 14:01:41.000000000","message":"I thought I\u0027d look to see where else dir_permission is being used. This has 0o755; the other uses 0o777. I think, if possible, we should be consistent. (And my other thought is to change the config itself to default to one of these values but not sure what others think of that.)","commit_id":"71f0b5eeaad68c8d2292a66c5c5f86a725e9c2e8"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"17e7a3dbbd68aa475de7b80076b3331e5f30e648","unresolved":true,"context_lines":[{"line_number":1296,"context_line":"                try:"},{"line_number":1297,"context_line":"                    os.makedirs("},{"line_number":1298,"context_line":"                        os.path.join(base_path, head),"},{"line_number":1299,"context_line":"                        mode\u003dCONF.pxe.dir_permission or 0o755, exist_ok\u003dTrue)"},{"line_number":1300,"context_line":"                except OSError as e:"},{"line_number":1301,"context_line":"                    msg \u003d (\u0027Failed to create supplied directories in \u0027"},{"line_number":1302,"context_line":"                           \u0027asset copy paths. Error: %s\u0027) % e"}],"source_content_type":"text/x-python","patch_set":4,"id":"4b40f373_22b69a2c","line":1299,"in_reply_to":"c6038bbd_c717fd2f","updated":"2021-11-10 20:11:22.000000000","message":"so 0o777 seems... wrong to me. Because that leaves assets world writable. 755 is relatively sane for what we use files for.","commit_id":"71f0b5eeaad68c8d2292a66c5c5f86a725e9c2e8"}],"ironic/conf/pxe.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"bb8f1f2c59fc51d77625b39745bfd5ace30444f1","unresolved":true,"context_lines":[{"line_number":109,"context_line":"               help\u003d_(\u0027The permission which is used on files created as part \u0027"},{"line_number":110,"context_line":"                      \u0027of configuration and setup of file assets for PXE \u0027"},{"line_number":111,"context_line":"                      \u0027based operations. The system default umask is masked \u0027"},{"line_number":112,"context_line":"                      \u0027out on the specified value. Defaults to a value of \u0027"},{"line_number":113,"context_line":"                      \u00270o644. This value must be specified as an octal \u0027"},{"line_number":114,"context_line":"                      \u0027representation. For example: 0o644\u0027)),"},{"line_number":115,"context_line":"    cfg.StrOpt(\u0027pxe_bootfile_name\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"4fa3337d_dcf2b796","line":112,"updated":"2021-11-02 14:01:41.000000000","message":"Would you do me a favour and confirm this? I looked and this config seems to only be used with os.chmod() and the docs don\u0027t seem to indicate this ?: https://docs.python.org/3/library/os.html#os.chmod.","commit_id":"71f0b5eeaad68c8d2292a66c5c5f86a725e9c2e8"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"c90e1bdda4e2a81ec523d0d7cec9ae045041acbe","unresolved":true,"context_lines":[{"line_number":109,"context_line":"               help\u003d_(\u0027The permission which is used on files created as part \u0027"},{"line_number":110,"context_line":"                      \u0027of configuration and setup of file assets for PXE \u0027"},{"line_number":111,"context_line":"                      \u0027based operations. The system default umask is masked \u0027"},{"line_number":112,"context_line":"                      \u0027out on the specified value. Defaults to a value of \u0027"},{"line_number":113,"context_line":"                      \u00270o644. This value must be specified as an octal \u0027"},{"line_number":114,"context_line":"                      \u0027representation. For example: 0o644\u0027)),"},{"line_number":115,"context_line":"    cfg.StrOpt(\u0027pxe_bootfile_name\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"35e95ddc_4490e0a1","line":112,"in_reply_to":"2bed7830_cb2b5353","updated":"2021-12-01 21:22:21.000000000","message":"Ugh, os.makedir applies the umask to the supplied mode, os.chmod does not. I\u0027ll just remove the docs change from file_permission","commit_id":"71f0b5eeaad68c8d2292a66c5c5f86a725e9c2e8"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"17e7a3dbbd68aa475de7b80076b3331e5f30e648","unresolved":true,"context_lines":[{"line_number":109,"context_line":"               help\u003d_(\u0027The permission which is used on files created as part \u0027"},{"line_number":110,"context_line":"                      \u0027of configuration and setup of file assets for PXE \u0027"},{"line_number":111,"context_line":"                      \u0027based operations. The system default umask is masked \u0027"},{"line_number":112,"context_line":"                      \u0027out on the specified value. Defaults to a value of \u0027"},{"line_number":113,"context_line":"                      \u00270o644. This value must be specified as an octal \u0027"},{"line_number":114,"context_line":"                      \u0027representation. For example: 0o644\u0027)),"},{"line_number":115,"context_line":"    cfg.StrOpt(\u0027pxe_bootfile_name\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"2bed7830_cb2b5353","line":112,"in_reply_to":"4fa3337d_dcf2b796","updated":"2021-11-10 20:11:22.000000000","message":"Umask is a lower level behavior, I\u0027d maybe note it after?! At the same time, distros ship different default umasks... at least did years ago.","commit_id":"71f0b5eeaad68c8d2292a66c5c5f86a725e9c2e8"}]}