)]}'
{"nova/cmd/manage.py":[{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"472d35357503e9b2c5af3c8cb886be342a8bc85d","unresolved":false,"context_lines":[{"line_number":1490,"context_line":"            default_log_levels\u003dlogging.get_default_log_levels() +"},{"line_number":1491,"context_line":"            _EXTRA_DEFAULT_LOG_LEVELS)"},{"line_number":1492,"context_line":"        logging.setup(CONF, \"nova\")"},{"line_number":1493,"context_line":"    except cfg.ConfigFilesNotFoundError as e:"},{"line_number":1494,"context_line":"        cfg_files \u003d \", \".join(e.config_files)"},{"line_number":1495,"context_line":"        print(_(\"Failed to read configuration file(s): %s\") % cfg_files)"},{"line_number":1496,"context_line":"        sys.exit(1)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a8bc5a2_2c0cfb41","line":1493,"updated":"2016-09-22 07:39:30.000000000","message":"What now is the reason to handle this? Why is this better than default behavior, i.e. in ConfigFilesPermissionDeniedError case?","commit_id":"f542b4d6db391c2abc3273861873fe4886ce8cc5"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"730be4068613ad47dd03015cf35fc5a490dfac44","unresolved":false,"context_lines":[{"line_number":1490,"context_line":"            default_log_levels\u003dlogging.get_default_log_levels() +"},{"line_number":1491,"context_line":"            _EXTRA_DEFAULT_LOG_LEVELS)"},{"line_number":1492,"context_line":"        logging.setup(CONF, \"nova\")"},{"line_number":1493,"context_line":"    except cfg.ConfigFilesNotFoundError as e:"},{"line_number":1494,"context_line":"        cfg_files \u003d \", \".join(e.config_files)"},{"line_number":1495,"context_line":"        print(_(\"Failed to read configuration file(s): %s\") % cfg_files)"},{"line_number":1496,"context_line":"        sys.exit(1)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a95cdbc_b9f06168","line":1493,"in_reply_to":"1a95cdbc_7587bfb4","updated":"2016-10-10 15:48:10.000000000","message":"Hm, on a second thought, removing the try-except completely sounds reasonable to me. I\u0027m going to push ps2 with that.","commit_id":"f542b4d6db391c2abc3273861873fe4886ce8cc5"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"ae93bd4d94d0b58bdbb53f8832743917856e2bcc","unresolved":false,"context_lines":[{"line_number":1490,"context_line":"            default_log_levels\u003dlogging.get_default_log_levels() +"},{"line_number":1491,"context_line":"            _EXTRA_DEFAULT_LOG_LEVELS)"},{"line_number":1492,"context_line":"        logging.setup(CONF, \"nova\")"},{"line_number":1493,"context_line":"    except cfg.ConfigFilesNotFoundError as e:"},{"line_number":1494,"context_line":"        cfg_files \u003d \", \".join(e.config_files)"},{"line_number":1495,"context_line":"        print(_(\"Failed to read configuration file(s): %s\") % cfg_files)"},{"line_number":1496,"context_line":"        sys.exit(1)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a95cdbc_7587bfb4","line":1493,"in_reply_to":"1a95cdbc_843fc3b2","updated":"2016-10-10 15:28:42.000000000","message":"@Markus, the other exception is just an example. I guess oslo.config can raise a lot of different exceptions. Does it mean that we have to catch each exception, printing appropriate messages then?\n\nI do not think so, because this behavior is already implemented in Python itself and oslo.config. The last one prepares user-friendly message related to an error, while the first one prints it. What is the reason to make \"our own wheel\" in this case?\n\nIn other words i propose to remove try-except clause here at all. E.g. see neighboring https://github.com/openstack/nova/blob/master/nova/cmd/policy_check.py#L158","commit_id":"f542b4d6db391c2abc3273861873fe4886ce8cc5"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"18c6c1d2657a792613fe78d49cd92a0918987b35","unresolved":false,"context_lines":[{"line_number":1490,"context_line":"            default_log_levels\u003dlogging.get_default_log_levels() +"},{"line_number":1491,"context_line":"            _EXTRA_DEFAULT_LOG_LEVELS)"},{"line_number":1492,"context_line":"        logging.setup(CONF, \"nova\")"},{"line_number":1493,"context_line":"    except cfg.ConfigFilesNotFoundError as e:"},{"line_number":1494,"context_line":"        cfg_files \u003d \", \".join(e.config_files)"},{"line_number":1495,"context_line":"        print(_(\"Failed to read configuration file(s): %s\") % cfg_files)"},{"line_number":1496,"context_line":"        sys.exit(1)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a95cdbc_843fc3b2","line":1493,"in_reply_to":"1a95cdbc_e44eab95","updated":"2016-10-10 14:22:33.000000000","message":"@Feodor: \"log\" was the wrong choice of words here, it\u0027s indeed just a print to the screen. After looking at \"oslo.config\" at\n\nhttps://github.com/openstack/oslo.config/blob/0715a15f7cd009780d771596ac690334503bfe2c/oslo_config/cfg.py#L2295-L2299\n\nI see now what you mean. We should have another except block which checks for the permission denial, right? Something like this:\n\n    except cfg.ConfigFilesNotFoundError as e:\n        cfg_files \u003d \", \".join(e.config_files)\n        print(_(\"Failed to read configuration file(s): %s\") % cfg_files)\n        sys.exit(1)\n    except cfg.ConfigFilesPermissionDeniedError as e:\n        cfg_files \u003d \", \".join(e.config_files)\n        print(_(\"No permission to read configuration file(s): %s\") % cfg_files)\n        print(_(\"Please re-run nova-manage as root.\"))\n        sys.exit(1)\n\nIs this what you mean?","commit_id":"f542b4d6db391c2abc3273861873fe4886ce8cc5"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"84f626a47dc2b11a2bd2b16e4e221371932a3a03","unresolved":false,"context_lines":[{"line_number":1490,"context_line":"            default_log_levels\u003dlogging.get_default_log_levels() +"},{"line_number":1491,"context_line":"            _EXTRA_DEFAULT_LOG_LEVELS)"},{"line_number":1492,"context_line":"        logging.setup(CONF, \"nova\")"},{"line_number":1493,"context_line":"    except cfg.ConfigFilesNotFoundError as e:"},{"line_number":1494,"context_line":"        cfg_files \u003d \", \".join(e.config_files)"},{"line_number":1495,"context_line":"        print(_(\"Failed to read configuration file(s): %s\") % cfg_files)"},{"line_number":1496,"context_line":"        sys.exit(1)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a95cdbc_e44eab95","line":1493,"in_reply_to":"1a95cdbc_f00e66e4","updated":"2016-10-04 16:04:41.000000000","message":"@Markus, but we log nothing here, just print to screen. The same does the default Python handler, does not it? Moreover standard ConfigFilesNotFoundError message contains the list of absent files.\n\nOtherwise why do we catch this exception only, but not ConfigFilesPermissionDeniedError, which means \u0027failed to read\u0027 as well?\n\nI think this catching was needed when we was trying to do something else in the case of error. But now, when we do nothing special (even logging) we probably do not need this block of code at all.\n\nDo i miss something?","commit_id":"f542b4d6db391c2abc3273861873fe4886ce8cc5"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"cb8505655c474214e5e0207f0996ce554a4b1d1a","unresolved":false,"context_lines":[{"line_number":1490,"context_line":"            default_log_levels\u003dlogging.get_default_log_levels() +"},{"line_number":1491,"context_line":"            _EXTRA_DEFAULT_LOG_LEVELS)"},{"line_number":1492,"context_line":"        logging.setup(CONF, \"nova\")"},{"line_number":1493,"context_line":"    except cfg.ConfigFilesNotFoundError as e:"},{"line_number":1494,"context_line":"        cfg_files \u003d \", \".join(e.config_files)"},{"line_number":1495,"context_line":"        print(_(\"Failed to read configuration file(s): %s\") % cfg_files)"},{"line_number":1496,"context_line":"        sys.exit(1)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a95cdbc_f00e66e4","line":1493,"in_reply_to":"5a8bc5a2_2c0cfb41","updated":"2016-10-04 15:38:04.000000000","message":"@Feodor: I think it\u0027s done this way to log the information the exception gives us (see line 1494 `e.config_files`). The old code does its own check \n\n    if cfgfile and not os.access(cfgfile, os.R_OK)\n\nto get this information, which is superfluous because the exception does this already.\n\n(I\u0027m back from vacation, therefore the long wait for my response.)","commit_id":"f542b4d6db391c2abc3273861873fe4886ce8cc5"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"a8ff20c98d0ed1a8a25183b0b9d45767dcd09e93","unresolved":false,"context_lines":[{"line_number":1490,"context_line":"            default_log_levels\u003dlogging.get_default_log_levels() +"},{"line_number":1491,"context_line":"            _EXTRA_DEFAULT_LOG_LEVELS)"},{"line_number":1492,"context_line":"        logging.setup(CONF, \"nova\")"},{"line_number":1493,"context_line":"    except cfg.ConfigFilesNotFoundError as e:"},{"line_number":1494,"context_line":"        cfg_files \u003d \", \".join(e.config_files)"},{"line_number":1495,"context_line":"        print(_(\"Failed to read configuration file(s): %s\") % cfg_files)"},{"line_number":1496,"context_line":"        sys.exit(1)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a8bc5a2_77f170cd","line":1493,"in_reply_to":"5a8bc5a2_2c0cfb41","updated":"2016-09-22 07:46:01.000000000","message":"i.e. -\u003e e.g.","commit_id":"f542b4d6db391c2abc3273861873fe4886ce8cc5"}]}