)]}'
{"swift/common/daemon.py":[{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"40d533608bee99c37aa4920af1b785bcc9632264","unresolved":false,"context_lines":[{"line_number":79,"context_line":"                              log_name\u003dkwargs.get(\u0027log_name\u0027))"},{"line_number":80,"context_line":"    except ValueError as ex:"},{"line_number":81,"context_line":"        print (ex)"},{"line_number":82,"context_line":"        sys.exit(1)"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    # once on command line (i.e. daemonize\u003dfalse) will over-ride config"},{"line_number":85,"context_line":"    once \u003d once or not utils.config_true_value(conf.get(\u0027daemonize\u0027, \u0027true\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a629dbe_4f937a4f","line":82,"updated":"2016-11-09 14:01:19.000000000","message":"I think a relevant message can be printed with sys.exit like:\n\nsys.exit(\u0027relevant value error message\u0027)","commit_id":"5430a3fc67823f60060a8aa4107042854ceb4f69"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8ff83df2fca05c16ef44bdb4e49e69767894a21e","unresolved":false,"context_lines":[{"line_number":81,"context_line":"    except ValueError as ex:"},{"line_number":82,"context_line":"        # The message will be printed to stderr"},{"line_number":83,"context_line":"        # and results in an exit code of 1."},{"line_number":84,"context_line":"        sys.exit(ex.message)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    # once on command line (i.e. daemonize\u003dfalse) will over-ride config"},{"line_number":87,"context_line":"    once \u003d once or not utils.config_true_value(conf.get(\u0027daemonize\u0027, \u0027true\u0027))"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a6eadb0_8485b4ae","line":84,"updated":"2016-12-13 18:43:36.000000000","message":"this makes sense","commit_id":"cdb3dd641d95c47421191c030d11dbdaa16bb743"}],"swift/common/middleware/crypto/keymaster.py":[{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"40d533608bee99c37aa4920af1b785bcc9632264","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                conf \u003d readconf(keymaster_config_path, \u0027keymaster\u0027)"},{"line_number":125,"context_line":"            except ValueError as ex:"},{"line_number":126,"context_line":"                print (ex)"},{"line_number":127,"context_line":"                sys.exit(1)"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        self.root_secret \u003d conf.get(\u0027encryption_root_secret\u0027)"},{"line_number":130,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a629dbe_0f9c2268","line":127,"updated":"2016-11-09 14:01:19.000000000","message":"I think a relevant message can be printed with sys.exit like:\n\nsys.exit(\u0027relevant value error message\u0027)\n\nAnd a change like this could be asserted with something like: https://github.com/openstack/swift/blob/master/test/unit/common/middleware/crypto/test_keymaster.py#L207","commit_id":"5430a3fc67823f60060a8aa4107042854ceb4f69"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8ff83df2fca05c16ef44bdb4e49e69767894a21e","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                                 \u0027are other config options specified!\u0027)"},{"line_number":123,"context_line":"            try:"},{"line_number":124,"context_line":"                conf \u003d readconf(keymaster_config_path, \u0027keymaster\u0027)"},{"line_number":125,"context_line":"            except ValueError as ex:"},{"line_number":126,"context_line":"                # The message will be printed to stderr"},{"line_number":127,"context_line":"                # and results in an exit code of 1."},{"line_number":128,"context_line":"                sys.exit(ex.message)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a6eadb0_a1c9c27a","line":125,"updated":"2016-12-13 18:43:36.000000000","message":"here I think it would be ok to just let the ValueError propagate because that would be consistent with other places in this __init__ where ValueError is raised e.g. line 137\n\nIf we *shoud* call sys.exit then we should be consistent and catch any ValueError raised from the __init__ and call sys.exit in one place in the except clause. But I am inclined NOT to do that - just raise ValueError here and let whatever is loading the middleware handle it. \n\nThe error message will be displayed whichever way.","commit_id":"cdb3dd641d95c47421191c030d11dbdaa16bb743"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"330e8e13e692838a1e4f3c671dc276a472dc33fe","unresolved":false,"context_lines":[{"line_number":125,"context_line":"            except ValueError as ex:"},{"line_number":126,"context_line":"                # The message will be printed to stderr"},{"line_number":127,"context_line":"                # and results in an exit code of 1."},{"line_number":128,"context_line":"                sys.exit(ex.message)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        self.root_secret \u003d conf.get(\u0027encryption_root_secret\u0027)"},{"line_number":131,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a6eadb0_64d25802","line":128,"updated":"2016-12-13 18:34:03.000000000","message":"I think it\u0027s best to just let the exception bubble up here, similar to how we\u0027ll explicitly raise a ValueError if both encryption_root_secret and keymaster_config_path are set.","commit_id":"cdb3dd641d95c47421191c030d11dbdaa16bb743"}],"swift/common/utils.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"330e8e13e692838a1e4f3c671dc276a472dc33fe","unresolved":false,"context_lines":[{"line_number":2440,"context_line":"        else:"},{"line_number":2441,"context_line":"            print(_(\"Unable to find %(section)s config section in %(conf)s\") %"},{"line_number":2442,"context_line":"                  {\u0027section\u0027: section_name, \u0027conf\u0027: conf_path})"},{"line_number":2443,"context_line":"            sys.exit(1)"},{"line_number":2444,"context_line":"        if \"log_name\" not in conf:"},{"line_number":2445,"context_line":"            if log_name is not None:"},{"line_number":2446,"context_line":"                conf[\u0027log_name\u0027] \u003d log_name"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a6eadb0_6427b812","side":"PARENT","line":2443,"updated":"2016-12-13 18:34:03.000000000","message":"Yeah, that\u0027s definitely an impolite way to handle failure :-(","commit_id":"a30351bdfc60f827979c5afe76ad40bf3a2b05c3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8ff83df2fca05c16ef44bdb4e49e69767894a21e","unresolved":false,"context_lines":[{"line_number":2417,"context_line":"    :param defaults: dict of default values to pre-populate the config with"},{"line_number":2418,"context_line":"    :returns: dict of config items"},{"line_number":2419,"context_line":"    :raises: ValueError if section_name not exists and"},{"line_number":2420,"context_line":"             triggers a SystemExit if the file read failed."},{"line_number":2421,"context_line":"    \"\"\""},{"line_number":2422,"context_line":"    if defaults is None:"},{"line_number":2423,"context_line":"        defaults \u003d {}"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a6eadb0_61140a44","line":2420,"updated":"2016-12-13 18:43:36.000000000","message":"I think the convention is to have a separate :raises \u003cexc\u003e: line for each exception type, and have the exception type inside the : delimiters http://www.sphinx-doc.org/en/1.5.1/domains.html#info-field-lists","commit_id":"cdb3dd641d95c47421191c030d11dbdaa16bb743"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"330e8e13e692838a1e4f3c671dc276a472dc33fe","unresolved":false,"context_lines":[{"line_number":2435,"context_line":"            success \u003d c.read(conf_path)"},{"line_number":2436,"context_line":"        if not success:"},{"line_number":2437,"context_line":"            print(_(\"Unable to read config from %s\") % conf_path)"},{"line_number":2438,"context_line":"            sys.exit(1)"},{"line_number":2439,"context_line":"    if section_name:"},{"line_number":2440,"context_line":"        if c.has_section(section_name):"},{"line_number":2441,"context_line":"            conf \u003d dict(c.items(section_name))"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a6eadb0_e4602838","line":2438,"updated":"2016-12-13 18:34:03.000000000","message":"Can we clean up this one, too? I\u0027d be OK with doing it as a follow-up, but since we\u0027re already here...","commit_id":"cdb3dd641d95c47421191c030d11dbdaa16bb743"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"330e8e13e692838a1e4f3c671dc276a472dc33fe","unresolved":false,"context_lines":[{"line_number":2440,"context_line":"        if c.has_section(section_name):"},{"line_number":2441,"context_line":"            conf \u003d dict(c.items(section_name))"},{"line_number":2442,"context_line":"        else:"},{"line_number":2443,"context_line":"            raise ValueError("},{"line_number":2444,"context_line":"                _(\"Unable to find %(section)s config section in %(conf)s\") %"},{"line_number":2445,"context_line":"                {\u0027section\u0027: section_name, \u0027conf\u0027: conf_path})"},{"line_number":2446,"context_line":"        if \"log_name\" not in conf:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a6eadb0_8478542d","line":2443,"updated":"2016-12-13 18:34:03.000000000","message":"ConfigParser.NoSectionError seems more appropriate.","commit_id":"cdb3dd641d95c47421191c030d11dbdaa16bb743"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8ff83df2fca05c16ef44bdb4e49e69767894a21e","unresolved":false,"context_lines":[{"line_number":2440,"context_line":"        if c.has_section(section_name):"},{"line_number":2441,"context_line":"            conf \u003d dict(c.items(section_name))"},{"line_number":2442,"context_line":"        else:"},{"line_number":2443,"context_line":"            raise ValueError("},{"line_number":2444,"context_line":"                _(\"Unable to find %(section)s config section in %(conf)s\") %"},{"line_number":2445,"context_line":"                {\u0027section\u0027: section_name, \u0027conf\u0027: conf_path})"},{"line_number":2446,"context_line":"        if \"log_name\" not in conf:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a6eadb0_2f2119a0","line":2443,"in_reply_to":"1a6eadb0_8478542d","updated":"2016-12-13 18:43:36.000000000","message":"Not sure I agree, that would be leaking implementation detail out of this function which would need to be cleaned up if we ever were to use a different config parsing library under the hood","commit_id":"cdb3dd641d95c47421191c030d11dbdaa16bb743"}],"swift/obj/auditor.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"330e8e13e692838a1e4f3c671dc276a472dc33fe","unresolved":false,"context_lines":[{"line_number":54,"context_line":"            # configure for their replicator"},{"line_number":55,"context_line":"            replicator_config \u003d readconf(self.conf[\u0027__file__\u0027],"},{"line_number":56,"context_line":"                                         \u0027object-replicator\u0027)"},{"line_number":57,"context_line":"        except (KeyError, SystemExit, ValueError):"},{"line_number":58,"context_line":"            # if we can\u0027t parse the real config (generally a KeyError on"},{"line_number":59,"context_line":"            # __file__, or ValueError on no object-replicator section) we use"},{"line_number":60,"context_line":"            # a very conservative default for rsync_timeout"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a6eadb0_04ba84c0","line":57,"range":{"start_line":57,"start_character":26,"end_line":57,"end_character":36},"updated":"2016-12-13 18:34:03.000000000","message":"Do we still need to catch SystemExit?","commit_id":"cdb3dd641d95c47421191c030d11dbdaa16bb743"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8ff83df2fca05c16ef44bdb4e49e69767894a21e","unresolved":false,"context_lines":[{"line_number":54,"context_line":"            # configure for their replicator"},{"line_number":55,"context_line":"            replicator_config \u003d readconf(self.conf[\u0027__file__\u0027],"},{"line_number":56,"context_line":"                                         \u0027object-replicator\u0027)"},{"line_number":57,"context_line":"        except (KeyError, SystemExit, ValueError):"},{"line_number":58,"context_line":"            # if we can\u0027t parse the real config (generally a KeyError on"},{"line_number":59,"context_line":"            # __file__, or ValueError on no object-replicator section) we use"},{"line_number":60,"context_line":"            # a very conservative default for rsync_timeout"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a6eadb0_4f72d5ae","line":57,"range":{"start_line":57,"start_character":26,"end_line":57,"end_character":36},"in_reply_to":"1a6eadb0_04ba84c0","updated":"2016-12-13 18:43:36.000000000","message":"it\u0027s raised if the file can\u0027t be read (which is very unlikely), but you suggested that could also be changed from a SystemExit so maybe this can go away here","commit_id":"cdb3dd641d95c47421191c030d11dbdaa16bb743"}],"test/__init__.py":[{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"5ca16ff0b79765eab060df1bb9a27ea1aed452f0","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        elif not os.access(config_file, os.R_OK):"},{"line_number":69,"context_line":"            print(\u0027Unable to read test config %s - permission denied\u0027"},{"line_number":70,"context_line":"                  % config_file, file\u003dsys.stderr)"},{"line_number":71,"context_line":"    except ValueError:"},{"line_number":72,"context_line":"        print(\u0027Unable to read test config %s - section %s not found\u0027"},{"line_number":73,"context_line":"              % (config_file, section_name), file\u003dsys.stderr)"},{"line_number":74,"context_line":"    return config"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a6eadb0_82fa9673","line":71,"updated":"2016-12-15 16:22:39.000000000","message":"just do \u0027except ValueError as e\u0027 and then print e?","commit_id":"ee722dcbc417dba5bdaf7808ae5e11b52d62a12e"}]}