)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"6fb8cba3aa8c27d4e65792943a7dace48eafddd6","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yuriy Nesenenko \u003cynesenenko@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-02-20 15:10:18 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix \u0027cinder-manage --config-dir /existing-dir db sync\u0027"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Command \u0027cinder-manage --config-dir /existing-dir is silently ignored"},{"line_number":10,"context_line":"if /existing-dir exists but doesn\u0027t contain config files."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba7be1f8_c1e733e3","line":7,"updated":"2015-02-20 13:24:07.000000000","message":"\"Handle incorrect \u0027--config-dir\u0027 param\" will be enough.","commit_id":"5c6462b8ad4a860c769c69984431da686ecc69f9"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"6fb8cba3aa8c27d4e65792943a7dace48eafddd6","unresolved":false,"context_lines":[{"line_number":11,"context_line":"This related to bug #1423169 in oslo_config/cfg.py"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: Iee3fbd2b01de11124788e2c1df5c456c4010d234"},{"line_number":14,"context_line":"Closes-Bug: #1255354"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba7be1f8_21c72f47","line":14,"updated":"2015-02-20 13:24:07.000000000","message":"Please, add Related-Bug from oslo.config and mark it as WIP until oslo.config will be updated","commit_id":"5c6462b8ad4a860c769c69984431da686ecc69f9"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8ad10cb9f0ab6ea9186f0c906298dbd359b13254","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yuriy Nesenenko \u003cynesenenko@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-06-09 14:20:59 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Handle incorrect \u0027--config-dir\u0027 param"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Command \u0027cinder-manage --config-dir /existing-dir is silently ignored"},{"line_number":10,"context_line":"if the /existing-dir exists but doesn\u0027t contain config files."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"fa32b979_0f5663d2","line":7,"updated":"2015-06-15 13:48:07.000000000","message":"Commit message is not correct, since incorrect --config-dir is handled by Oslo config and here we are trying to handle something completely different.","commit_id":"d2a1fe5f05cfdbc03aa9aef2951bb5235370ba4d"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"352e440564f35e3b516402ceb71880ad0a98d3a4","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yuriy Nesenenko \u003cynesenenko@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-06-09 14:20:59 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Handle incorrect \u0027--config-dir\u0027 param"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Command \u0027cinder-manage --config-dir /existing-dir is silently ignored"},{"line_number":10,"context_line":"if the /existing-dir exists but doesn\u0027t contain config files."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"fa32b979_1b1ea5ba","line":7,"in_reply_to":"fa32b979_0f5663d2","updated":"2015-06-15 14:39:24.000000000","message":"cfg.py makes no attempt to verify the existence or state of --config-dir . As a result, it puts together the glob for files (I.E. /invalid-dir/*.conf) and tries to get a list of files in there. The list is empty so it just moves on. When --config-dir /existing-dir, which doesn\u0027t contain config files, the command \u0027cinder-manage --config-dir /existing-dir db sync\u0027 is silently ignored. An example command like that exhibits this problem is \u0027cinder-manage --config-dir /home db sync\u0027. As you see, the directory /home doesn\u0027t contain config files at all but it is silently ignored.","commit_id":"d2a1fe5f05cfdbc03aa9aef2951bb5235370ba4d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8027fe8f6ac8c8c5364ce9eb2c733970e62b9ad4","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yuriy Nesenenko \u003cynesenenko@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-06-09 14:20:59 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Handle incorrect \u0027--config-dir\u0027 param"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Command \u0027cinder-manage --config-dir /existing-dir is silently ignored"},{"line_number":10,"context_line":"if the /existing-dir exists but doesn\u0027t contain config files."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"fa32b979_6693945c","line":7,"in_reply_to":"fa32b979_1b1ea5ba","updated":"2015-06-15 14:51:56.000000000","message":"It is not being ignored, since all .conf files will be handled. There could be 100 or 0 config files. That is not relevant. There is no reason to fail just because it didn\u0027t find any.\n\nYou could be setting your defaults using --config-file and then setting --config-dir to a directory that it\u0027s usually empty except in rare occasions when you actually have some config files in there. And in this case it should not give any errors, at least that\u0027s my opinion.","commit_id":"d2a1fe5f05cfdbc03aa9aef2951bb5235370ba4d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8701b25d9d3fc9126c30e052471aa44058ce0a41","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yuriy Nesenenko \u003cynesenenko@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-06-09 14:20:59 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Handle incorrect \u0027--config-dir\u0027 param"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Command \u0027cinder-manage --config-dir /existing-dir is silently ignored"},{"line_number":10,"context_line":"if the /existing-dir exists but doesn\u0027t contain config files."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"fa32b979_56ae3f2f","line":7,"in_reply_to":"fa32b979_6693945c","updated":"2015-06-15 15:55:56.000000000","message":"You are right, Cinder still need to fix the --config-dir issue, but I think at this point is more correct to say that we need to catch ConfigDirNotFoundError for those cases where the directory doesn\u0027t exist.","commit_id":"d2a1fe5f05cfdbc03aa9aef2951bb5235370ba4d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8701b25d9d3fc9126c30e052471aa44058ce0a41","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Handle incorrect \u0027--config-dir\u0027 param"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Command \u0027cinder-manage --config-dir /existing-dir is silently ignored"},{"line_number":10,"context_line":"if the /existing-dir exists but doesn\u0027t contain config files."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Iee3fbd2b01de11124788e2c1df5c456c4010d234"},{"line_number":13,"context_line":"Closes-Bug: #1255354"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"fa32b979_967287e4","line":10,"updated":"2015-06-15 15:55:56.000000000","message":"I don\u0027t think this should be part of this patch since it is not related to the bug and I don\u0027t even think that\u0027s a bug.","commit_id":"d2a1fe5f05cfdbc03aa9aef2951bb5235370ba4d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8ad10cb9f0ab6ea9186f0c906298dbd359b13254","unresolved":false,"context_lines":[{"line_number":10,"context_line":"if the /existing-dir exists but doesn\u0027t contain config files."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Iee3fbd2b01de11124788e2c1df5c456c4010d234"},{"line_number":13,"context_line":"Closes-Bug: #1255354"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"fa32b979_6f112f52","line":13,"updated":"2015-06-15 13:48:07.000000000","message":"Incorrect bug reference. Referenced bug is about directory being incorrect, which has been fixed in oslo_config.","commit_id":"d2a1fe5f05cfdbc03aa9aef2951bb5235370ba4d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8701b25d9d3fc9126c30e052471aa44058ce0a41","unresolved":false,"context_lines":[{"line_number":10,"context_line":"if the /existing-dir exists but doesn\u0027t contain config files."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Iee3fbd2b01de11124788e2c1df5c456c4010d234"},{"line_number":13,"context_line":"Closes-Bug: #1255354"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"fa32b979_36785bc4","line":13,"in_reply_to":"fa32b979_6f112f52","updated":"2015-06-15 15:55:56.000000000","message":"Bug reference is right.","commit_id":"d2a1fe5f05cfdbc03aa9aef2951bb5235370ba4d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"73033e5e1044f6fdfd02ef9e7ee0c6a278192886","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Handle incorrect \u0027--config-dir\u0027 param"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If the --config-dir directory doesn\u0027t exist, ConfigDirNotFoundError"},{"line_number":10,"context_line":"should be caught. Command \u0027cinder-manage --config-dir /existing-dir"},{"line_number":11,"context_line":"is silently ignored if the /existing-dir exists but doesn\u0027t contain"},{"line_number":12,"context_line":"config files. So it\u0027s used config files by default."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"fa32b979_34bfcffc","line":10,"updated":"2015-06-16 09:52:45.000000000","message":"We should not talk about the existing dir without .conf files in this commit message.\n\nInstead I suggest you mention the error that you would get without this patch (an extract from the paste link from your comment) and how this patch fixes this.","commit_id":"b235f42338e0c7a065945c83827004aa7dc46b52"}],"cinder/cmd/manage.py":[{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"6fb8cba3aa8c27d4e65792943a7dace48eafddd6","unresolved":false,"context_lines":[{"line_number":544,"context_line":"        CONF(sys.argv[1:], project\u003d\u0027cinder\u0027,"},{"line_number":545,"context_line":"             version\u003dversion.version_string())"},{"line_number":546,"context_line":"        logging.setup(\"cinder\")"},{"line_number":547,"context_line":"    except cfg.ConfigDirNotFoundError as detail:"},{"line_number":548,"context_line":"        print(detail)"},{"line_number":549,"context_line":"        sys.exit(2)"},{"line_number":550,"context_line":"    # If config files are not found in the requested directory."}],"source_content_type":"text/x-python","patch_set":2,"id":"ba7be1f8_81b61bdb","line":547,"updated":"2015-02-20 13:24:07.000000000","message":"Please, use more user-friendly variable names","commit_id":"5c6462b8ad4a860c769c69984431da686ecc69f9"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"6fb8cba3aa8c27d4e65792943a7dace48eafddd6","unresolved":false,"context_lines":[{"line_number":549,"context_line":"        sys.exit(2)"},{"line_number":550,"context_line":"    # If config files are not found in the requested directory."},{"line_number":551,"context_line":"    except cfg.ConfigFilesNotFound as detail2:"},{"line_number":552,"context_line":"        print(detail2)"},{"line_number":553,"context_line":"        sys.exit(2)"},{"line_number":554,"context_line":"    except cfg.ConfigFilesNotFoundError:"},{"line_number":555,"context_line":"        cfgfile \u003d CONF.config_file[-1] if CONF.config_file else None"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba7be1f8_41ad0388","line":552,"updated":"2015-02-20 13:24:07.000000000","message":"The same as above","commit_id":"5c6462b8ad4a860c769c69984431da686ecc69f9"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"e759495381231f69127e64a4e0d6c46cd181719b","unresolved":false,"context_lines":[{"line_number":548,"context_line":"        print(details)"},{"line_number":549,"context_line":"        sys.exit(2)"},{"line_number":550,"context_line":"    # If config files are not found in the requested directory."},{"line_number":551,"context_line":"    except cfg.ConfigFilesNotFound as error_details:"},{"line_number":552,"context_line":"        print(error_details)"},{"line_number":553,"context_line":"        sys.exit(2)"},{"line_number":554,"context_line":"    except cfg.ConfigFilesNotFoundError:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba7be1f8_583677e8","line":551,"updated":"2015-02-24 07:10:46.000000000","message":"There is no ConfigFilesNotFound exception defined in oslo_config, unless your change to bug 1423169 will add this?  What is the difference between ConfigFilesNotFound and ConfigFilesNotFoundError.  Judging by the name these two are confusing.","commit_id":"cdbf9517a6223ae3b00314b50934c21bb66ed1a9"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"e735a6d3deb348e5ccb1ada1f0b7f59ab2a9728b","unresolved":false,"context_lines":[{"line_number":548,"context_line":"        print(details)"},{"line_number":549,"context_line":"        sys.exit(2)"},{"line_number":550,"context_line":"    # If config files are not found in the requested directory."},{"line_number":551,"context_line":"    except cfg.ConfigFilesNotFound as error_details:"},{"line_number":552,"context_line":"        print(error_details)"},{"line_number":553,"context_line":"        sys.exit(2)"},{"line_number":554,"context_line":"    except cfg.ConfigFilesNotFoundError:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba7be1f8_1b731681","line":551,"in_reply_to":"ba7be1f8_583677e8","updated":"2015-02-24 09:40:41.000000000","message":"ConfigFilesNotFound raises if config files are not found in the requested directory --config-dir where it exists but doesn\u0027t contain config files. It\u0027s done because cfg.py doesn\u0027t check if list_config_files is empty.  ConfigFilesNotFound returns the name of the requested directory.\nConfigFilesNotFoundError returns names of config files.","commit_id":"cdbf9517a6223ae3b00314b50934c21bb66ed1a9"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"ecabdd7c4a5fea5528b5d33a2b13fab931fb7964","unresolved":false,"context_lines":[{"line_number":547,"context_line":"        CONF(sys.argv[1:], project\u003d\u0027cinder\u0027,"},{"line_number":548,"context_line":"             version\u003dversion.version_string())"},{"line_number":549,"context_line":"        logging.setup(CONF, \"cinder\")"},{"line_number":550,"context_line":"    except cfg.ConfigDirNotFoundError as details:"},{"line_number":551,"context_line":"        print(details)"},{"line_number":552,"context_line":"        sys.exit(2)"},{"line_number":553,"context_line":"    # If config files are not found in the requested directory."}],"source_content_type":"text/x-python","patch_set":5,"id":"7a016987_345e00ea","line":550,"updated":"2015-05-27 10:43:20.000000000","message":"Please, print more user friendly message","commit_id":"597945a0bf011dc65bdf462543da82bae6dece8d"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"427072b05a6b4905dfbcc815f625fb7c8de3c22d","unresolved":false,"context_lines":[{"line_number":547,"context_line":"        CONF(sys.argv[1:], project\u003d\u0027cinder\u0027,"},{"line_number":548,"context_line":"             version\u003dversion.version_string())"},{"line_number":549,"context_line":"        logging.setup(CONF, \"cinder\")"},{"line_number":550,"context_line":"        if CONF.config_file:"},{"line_number":551,"context_line":"            cfgfile \u003d os.path.basename(CONF.config_file[-1])"},{"line_number":552,"context_line":"            cfgfile \u003d os.path.join(CONF.config_dir, cfgfile)"},{"line_number":553,"context_line":"            if not os.path.exists(cfgfile):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3afb71cf_183135ed","line":550,"updated":"2015-06-04 20:51:58.000000000","message":"This code is confusing a bit because we have .ConfigFilesNotFoundError exception handler. Please add comment why do we need it here","commit_id":"c499d8a02a69e9c406d09b8fefe3020950d655d5"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"427072b05a6b4905dfbcc815f625fb7c8de3c22d","unresolved":false,"context_lines":[{"line_number":548,"context_line":"             version\u003dversion.version_string())"},{"line_number":549,"context_line":"        logging.setup(CONF, \"cinder\")"},{"line_number":550,"context_line":"        if CONF.config_file:"},{"line_number":551,"context_line":"            cfgfile \u003d os.path.basename(CONF.config_file[-1])"},{"line_number":552,"context_line":"            cfgfile \u003d os.path.join(CONF.config_dir, cfgfile)"},{"line_number":553,"context_line":"            if not os.path.exists(cfgfile):"},{"line_number":554,"context_line":"                print(_(\"Failed to find config files in the directory: %s\") %"}],"source_content_type":"text/x-python","patch_set":7,"id":"3afb71cf_6d0ced42","line":551,"updated":"2015-06-04 20:51:58.000000000","message":"Why do we check only the last config file?","commit_id":"c499d8a02a69e9c406d09b8fefe3020950d655d5"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"427072b05a6b4905dfbcc815f625fb7c8de3c22d","unresolved":false,"context_lines":[{"line_number":551,"context_line":"            cfgfile \u003d os.path.basename(CONF.config_file[-1])"},{"line_number":552,"context_line":"            cfgfile \u003d os.path.join(CONF.config_dir, cfgfile)"},{"line_number":553,"context_line":"            if not os.path.exists(cfgfile):"},{"line_number":554,"context_line":"                print(_(\"Failed to find config files in the directory: %s\") %"},{"line_number":555,"context_line":"                      CONF.config_dir)"},{"line_number":556,"context_line":"                sys.exit(2)"},{"line_number":557,"context_line":"    except cfg.ConfigDirNotFoundError as details:"}],"source_content_type":"text/x-python","patch_set":7,"id":"3afb71cf_7813895d","line":554,"updated":"2015-06-04 20:51:58.000000000","message":"file","commit_id":"c499d8a02a69e9c406d09b8fefe3020950d655d5"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"427072b05a6b4905dfbcc815f625fb7c8de3c22d","unresolved":false,"context_lines":[{"line_number":552,"context_line":"            cfgfile \u003d os.path.join(CONF.config_dir, cfgfile)"},{"line_number":553,"context_line":"            if not os.path.exists(cfgfile):"},{"line_number":554,"context_line":"                print(_(\"Failed to find config files in the directory: %s\") %"},{"line_number":555,"context_line":"                      CONF.config_dir)"},{"line_number":556,"context_line":"                sys.exit(2)"},{"line_number":557,"context_line":"    except cfg.ConfigDirNotFoundError as details:"},{"line_number":558,"context_line":"        print(_(\"Invalid directory: %s\") % details)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3afb71cf_f8fe5904","line":555,"updated":"2015-06-04 20:51:58.000000000","message":"Please, add config file name to the message","commit_id":"c499d8a02a69e9c406d09b8fefe3020950d655d5"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"1cbf38b16b718295a0cedb00afa820cdd4b14d65","unresolved":false,"context_lines":[{"line_number":547,"context_line":"        CONF(sys.argv[1:], project\u003d\u0027cinder\u0027,"},{"line_number":548,"context_line":"             version\u003dversion.version_string())"},{"line_number":549,"context_line":"        logging.setup(CONF, \"cinder\")"},{"line_number":550,"context_line":"    except cfg.ConfigDirNotFoundError as details:"},{"line_number":551,"context_line":"        print(_(\"Invalid directory: %s\") % details)"},{"line_number":552,"context_line":"        sys.exit(2)"},{"line_number":553,"context_line":"    except cfg.ConfigFilesNotFoundError:"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa32b979_3124575c","line":550,"updated":"2015-06-18 07:15:24.000000000","message":"Would it be OK to add a test for this ConfigDirNotFoundError case?","commit_id":"3d08d9821508cc6e8de6ab6301ab3ed23abf76ce"}],"cinder/tests/unit/test_cmd.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"32dc1c291f79a5fe59bd4cf66a3ebef8abeef703","unresolved":false,"context_lines":[{"line_number":709,"context_line":"        script_name \u003d \u0027cinder-manage\u0027"},{"line_number":710,"context_line":"        sys.argv \u003d [script_name, \u0027--config-dir\u0027, \u0027fake-dir\u0027]"},{"line_number":711,"context_line":"        config_opts_call.side_effect \u003d cfg.ConfigDirNotFoundError("},{"line_number":712,"context_line":"            mock.sentinel._namespace)"},{"line_number":713,"context_line":""},{"line_number":714,"context_line":"        with mock.patch(\u0027sys.stdout\u0027, new\u003dsix.StringIO()):"},{"line_number":715,"context_line":"            exit \u003d self.assertRaises(SystemExit, cinder_manage.main)"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_f6f01dcb","line":712,"updated":"2015-06-18 10:07:16.000000000","message":"This should be fake-dir and not the sentinel (unless you use sentinel in the call as well.","commit_id":"56f3be659ccb16022a2f04aa9defe9a188cd7ea6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"32dc1c291f79a5fe59bd4cf66a3ebef8abeef703","unresolved":false,"context_lines":[{"line_number":711,"context_line":"        config_opts_call.side_effect \u003d cfg.ConfigDirNotFoundError("},{"line_number":712,"context_line":"            mock.sentinel._namespace)"},{"line_number":713,"context_line":""},{"line_number":714,"context_line":"        with mock.patch(\u0027sys.stdout\u0027, new\u003dsix.StringIO()):"},{"line_number":715,"context_line":"            exit \u003d self.assertRaises(SystemExit, cinder_manage.main)"},{"line_number":716,"context_line":""},{"line_number":717,"context_line":"            self.assertTrue(register_cli_opt.called)"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_56cd4983","line":714,"updated":"2015-06-18 10:07:16.000000000","message":"You are not really checking that you get the error you are expecting. You could be getting exit code 2 for some other reason. We all know that\u0027s not the case now, but in the future if you don\u0027t do a strict check we could have regressions.\n\nYou should assign that mock to a variable and then check that it contains \"Invalid directory: Failed to read config file directory: fake-dir\".","commit_id":"56f3be659ccb16022a2f04aa9defe9a188cd7ea6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4cce46a9444d89135b15ed7c6768f39c767ad0fa","unresolved":false,"context_lines":[{"line_number":717,"context_line":"            config_opts_call.assert_called_once_with("},{"line_number":718,"context_line":"                sys.argv[1:], project\u003d\u0027cinder\u0027,"},{"line_number":719,"context_line":"                version\u003dversion.version_string())"},{"line_number":720,"context_line":"            self.assertTrue(fake_dir in fake_out.getvalue())"},{"line_number":721,"context_line":"            self.assertFalse(log_setup.called)"},{"line_number":722,"context_line":"            self.assertEqual(2, exit.code)"},{"line_number":723,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"fa32b979_5846cc7b","line":720,"updated":"2015-06-18 14:09:36.000000000","message":"I would prefer if you checked Invalid directory as well, because stdout could contain something different, like an exception mentioning that argument value.","commit_id":"f1962f8d5a714f97dd8c83f962e9d16420967e99"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"053a09684331e7910111ba68da433ce16457fbd6","unresolved":false,"context_lines":[{"line_number":719,"context_line":"                sys.argv[1:], project\u003d\u0027cinder\u0027,"},{"line_number":720,"context_line":"                version\u003dversion.version_string())"},{"line_number":721,"context_line":"            self.assertTrue(invalid_dir in fake_out.getvalue())"},{"line_number":722,"context_line":"            self.assertTrue(fake_dir in fake_out.getvalue())"},{"line_number":723,"context_line":"            self.assertFalse(log_setup.called)"},{"line_number":724,"context_line":"            self.assertEqual(2, exit.code)"},{"line_number":725,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"fa32b979_dc6cfed1","line":722,"updated":"2015-06-18 19:45:09.000000000","message":"Not important, but usually for these cases is better to use self.assertIn. On error will report better information.   ;-)","commit_id":"0491386b719bb810332c5b2632d1a9fbeb9e546a"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"976d4fed442d01dcd67a6ffa11f1d92f9bc786e7","unresolved":false,"context_lines":[{"line_number":718,"context_line":"            config_opts_call.assert_called_once_with("},{"line_number":719,"context_line":"                sys.argv[1:], project\u003d\u0027cinder\u0027,"},{"line_number":720,"context_line":"                version\u003dversion.version_string())"},{"line_number":721,"context_line":"            self.assertIn(invalid_dir, fake_out.getvalue())"},{"line_number":722,"context_line":"            self.assertFalse(log_setup.called)"},{"line_number":723,"context_line":"            self.assertEqual(2, exit.code)"},{"line_number":724,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"fa32b979_963ddcbd","line":721,"updated":"2015-06-19 09:37:19.000000000","message":"Why did you remove the fake_dir check?\n\n self.assertIn(fake_dir, fake_out.getvalue())","commit_id":"8c4d5780a0f80903ef9c292ef663e5dbd271fa97"}]}