)]}'
{"novaclient/shell.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"95a201a9aa607c7b6604e74c100215a530748aae","unresolved":false,"context_lines":[{"line_number":141,"context_line":"            return self.args.os_password"},{"line_number":142,"context_line":"        if self._validate_string(self.args.apikey):"},{"line_number":143,"context_line":"            return self.args.apikey"},{"line_number":144,"context_line":"        verify_pass \u003d utils.bool_from_str(os.getenv(\"OS_VERIFY_PASSWORD\"))"},{"line_number":145,"context_line":"        return self._prompt_password(verify_pass)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAM3%2F%2BsB8%3D","line":144,"updated":"2013-02-21 16:43:44.000000000","message":"Instead of switching to os.getenv(), switch this to utils.env()","commit_id":"1e1b9fe6eb66814aa378a2ceb4b68c48ce188090"},{"author":{"_account_id":6724,"name":"Vasyl Khomenko","email":"vasiliyk@yahoo-inc.com","username":"vkhomenko"},"change_message_id":"66b09992888c500dc3655534321711ad11672eba","unresolved":false,"context_lines":[{"line_number":141,"context_line":"            return self.args.os_password"},{"line_number":142,"context_line":"        if self._validate_string(self.args.apikey):"},{"line_number":143,"context_line":"            return self.args.apikey"},{"line_number":144,"context_line":"        verify_pass \u003d utils.bool_from_str(os.getenv(\"OS_VERIFY_PASSWORD\"))"},{"line_number":145,"context_line":"        return self._prompt_password(verify_pass)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAM3%2F%2Br70%3D","line":144,"in_reply_to":"AAAAM3%2F%2BsB8%3D","updated":"2013-02-22 13:21:47.000000000","message":"Done","commit_id":"1e1b9fe6eb66814aa378a2ceb4b68c48ce188090"}],"tests/test_shell.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"95a201a9aa607c7b6604e74c100215a530748aae","unresolved":false,"context_lines":[{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def test_no_username(self):"},{"line_number":112,"context_line":"        required \u003d \"You must provide a username\""},{"line_number":113,"context_line":"        del(os.environ[\u0027OS_USERNAME\u0027])"},{"line_number":114,"context_line":"        with mock.patch.object(novaclient.utils, \u0027isunauthenticated\u0027,"},{"line_number":115,"context_line":"                               return_value\u003dFalse):"},{"line_number":116,"context_line":"            self.assertRaisesRegexp("}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAM3%2F%2BsAg%3D","line":113,"updated":"2013-02-21 16:43:44.000000000","message":"1. \u0027del\u0027 is a statement, so I\u0027d slightly prefer this be formatted as such, i.e.:\n\n    del os.environ[\u0027OS_USERNAME\u0027]\n\n2. Deleting a dictionary key that is not set raises a KeyError, meaning that these tests are likely to fail unless all these environment variables are set.  Given that two tests delete OS_PASSWORD, this is a given.\n\n3. I do not like permanent modifications of any type in tests; you should instead patch the environment to eliminate the variables you wish to ensure are not set.","commit_id":"1e1b9fe6eb66814aa378a2ceb4b68c48ce188090"},{"author":{"_account_id":6724,"name":"Vasyl Khomenko","email":"vasiliyk@yahoo-inc.com","username":"vkhomenko"},"change_message_id":"66b09992888c500dc3655534321711ad11672eba","unresolved":false,"context_lines":[{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def test_no_username(self):"},{"line_number":112,"context_line":"        required \u003d \"You must provide a username\""},{"line_number":113,"context_line":"        del(os.environ[\u0027OS_USERNAME\u0027])"},{"line_number":114,"context_line":"        with mock.patch.object(novaclient.utils, \u0027isunauthenticated\u0027,"},{"line_number":115,"context_line":"                               return_value\u003dFalse):"},{"line_number":116,"context_line":"            self.assertRaisesRegexp("}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAM3%2F%2Br2M%3D","line":113,"in_reply_to":"AAAAM3%2F%2BsAg%3D","updated":"2013-02-22 13:21:47.000000000","message":"Done","commit_id":"1e1b9fe6eb66814aa378a2ceb4b68c48ce188090"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"95a201a9aa607c7b6604e74c100215a530748aae","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        with mock.patch.object(novaclient.utils, \u0027isunauthenticated\u0027,"},{"line_number":139,"context_line":"                               return_value\u003dFalse):"},{"line_number":140,"context_line":"            with mock.patch.object(sys, \u0027stdin\u0027, side_effect\u003dmock.MagicMock):"},{"line_number":141,"context_line":"                with mock.patch.object(getpass, \u0027getpass\u0027,"},{"line_number":142,"context_line":"                                       return_value\u003d\u0027entered_password\u0027):"},{"line_number":143,"context_line":"                    stdout, stderr \u003d self.shell(\u0027list\u0027)"},{"line_number":144,"context_line":"                    self.assertEquals((stdout + stderr), \u0027\\n\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAM3%2F%2BsAA%3D","line":141,"updated":"2013-02-21 16:43:44.000000000","message":"Given the depth of \u0027with\u0027 statements, I would greatly prefer you use the decorator syntax.  That will avoid excessive indentation and make it a little clearer exactly what you\u0027re patching out.  It will also make it easier to modify the patches, e.g. adding or removing one.  Also, for methods in modules that were not previously imported, consider using mock.patch() instead of mock.patch.object(); that way, you don\u0027t have to import those modules.","commit_id":"1e1b9fe6eb66814aa378a2ceb4b68c48ce188090"},{"author":{"_account_id":6724,"name":"Vasyl Khomenko","email":"vasiliyk@yahoo-inc.com","username":"vkhomenko"},"change_message_id":"66b09992888c500dc3655534321711ad11672eba","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        with mock.patch.object(novaclient.utils, \u0027isunauthenticated\u0027,"},{"line_number":139,"context_line":"                               return_value\u003dFalse):"},{"line_number":140,"context_line":"            with mock.patch.object(sys, \u0027stdin\u0027, side_effect\u003dmock.MagicMock):"},{"line_number":141,"context_line":"                with mock.patch.object(getpass, \u0027getpass\u0027,"},{"line_number":142,"context_line":"                                       return_value\u003d\u0027entered_password\u0027):"},{"line_number":143,"context_line":"                    stdout, stderr \u003d self.shell(\u0027list\u0027)"},{"line_number":144,"context_line":"                    self.assertEquals((stdout + stderr), \u0027\\n\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAM3%2F%2BqUk%3D","line":141,"in_reply_to":"AAAAM3%2F%2BsAA%3D","updated":"2013-02-22 13:21:47.000000000","message":"Done","commit_id":"1e1b9fe6eb66814aa378a2ceb4b68c48ce188090"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"ec48f381d0b051e7d0d57b349108ae36aa3237d1","unresolved":false,"context_lines":[{"line_number":111,"context_line":"                    \u0027 via either --os-username or env[OS_USERNAME]\u0027,)"},{"line_number":112,"context_line":"        env \u003d {\u0027OS_PASSWORD\u0027: \u0027password\u0027,"},{"line_number":113,"context_line":"               \u0027OS_TENANT_NAME\u0027: \u0027tenant_name\u0027,"},{"line_number":114,"context_line":"               \u0027OS_AUTH_URL\u0027: \u0027http://no.where\u0027}"},{"line_number":115,"context_line":"        self.make_env(env)"},{"line_number":116,"context_line":"        try:"},{"line_number":117,"context_line":"            self.shell(\u0027list\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAM3%2F%2BqEo%3D","line":114,"updated":"2013-02-22 16:23:36.000000000","message":"To reduce duplication, you could use something like:\n\n    env \u003d FAKE_ENV.copy()\n    del env[\u0027OS_USERNAME\u0027]\n\nYou could even encode that logic directly into make_env():\n\n    def make_env(self, *skip_vars):\n        for var, val in FAKE_ENV.items():\n            if var in skip_vars:\n                continue\n            self.useFixture(fixtures.EnvironmentVariable(var, val))\n\nThen here you would just need\n\n    self.make_env(\u0027OS_USERNAME\u0027)","commit_id":"3d7be328767648e46d6364b55e09640d0b32de20"},{"author":{"_account_id":6724,"name":"Vasyl Khomenko","email":"vasiliyk@yahoo-inc.com","username":"vkhomenko"},"change_message_id":"e4290142ae5a9f2fbd12e8bee72e8e0c79ed6e77","unresolved":false,"context_lines":[{"line_number":111,"context_line":"                    \u0027 via either --os-username or env[OS_USERNAME]\u0027,)"},{"line_number":112,"context_line":"        env \u003d {\u0027OS_PASSWORD\u0027: \u0027password\u0027,"},{"line_number":113,"context_line":"               \u0027OS_TENANT_NAME\u0027: \u0027tenant_name\u0027,"},{"line_number":114,"context_line":"               \u0027OS_AUTH_URL\u0027: \u0027http://no.where\u0027}"},{"line_number":115,"context_line":"        self.make_env(env)"},{"line_number":116,"context_line":"        try:"},{"line_number":117,"context_line":"            self.shell(\u0027list\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAM3%2F%2Bp1s%3D","line":114,"in_reply_to":"AAAAM3%2F%2BqEo%3D","updated":"2013-02-22 18:18:08.000000000","message":"You are right. Done.","commit_id":"3d7be328767648e46d6364b55e09640d0b32de20"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"ec48f381d0b051e7d0d57b349108ae36aa3237d1","unresolved":false,"context_lines":[{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        @mock.patch(\u0027sys.stdin\u0027, side_effect\u003dmock.MagicMock)"},{"line_number":160,"context_line":"        @mock.patch(\u0027getpass.getpass\u0027, return_value\u003d\u0027password\u0027)"},{"line_number":161,"context_line":"        def check(*args, **kwargs):"},{"line_number":162,"context_line":"            stdout, stderr \u003d self.shell(\u0027list\u0027)"},{"line_number":163,"context_line":"            self.assertEqual((stdout + stderr), \u0027\\n\u0027)"},{"line_number":164,"context_line":"        check()"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAM3%2F%2BqE4%3D","line":161,"updated":"2013-02-22 16:23:36.000000000","message":"I slightly prefer for the @mock.patch()\u0027s to go on the test method itself, i.e.:\n\n    @mock.patch(\u0027sys.stdin\u0027, side_effect\u003dmock.MagicMock)\n    @mock.patch(\u0027getpass.getpass\u0027, return_value\u003d\u0027password\u0027)\n    def test_password(self, mock_getpass, mock_stdin):\n        …\n\nThis is fine, however, so I\u0027m not going to down-vote just because of this…\n\n(Also, I\u0027m not sure you need the side_effect on the sys.stdin mock, but I could be wrong.)","commit_id":"3d7be328767648e46d6364b55e09640d0b32de20"},{"author":{"_account_id":6724,"name":"Vasyl Khomenko","email":"vasiliyk@yahoo-inc.com","username":"vkhomenko"},"change_message_id":"e4290142ae5a9f2fbd12e8bee72e8e0c79ed6e77","unresolved":false,"context_lines":[{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        @mock.patch(\u0027sys.stdin\u0027, side_effect\u003dmock.MagicMock)"},{"line_number":160,"context_line":"        @mock.patch(\u0027getpass.getpass\u0027, return_value\u003d\u0027password\u0027)"},{"line_number":161,"context_line":"        def check(*args, **kwargs):"},{"line_number":162,"context_line":"            stdout, stderr \u003d self.shell(\u0027list\u0027)"},{"line_number":163,"context_line":"            self.assertEqual((stdout + stderr), \u0027\\n\u0027)"},{"line_number":164,"context_line":"        check()"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAM3%2F%2Bp1E%3D","line":161,"in_reply_to":"AAAAM3%2F%2BqE4%3D","updated":"2013-02-22 18:18:08.000000000","message":"Done","commit_id":"3d7be328767648e46d6364b55e09640d0b32de20"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"befdb0e7faf6983f913baf29672be31b90913efa","unresolved":false,"context_lines":[{"line_number":2,"context_line":"import re"},{"line_number":3,"context_line":"import sys"},{"line_number":4,"context_line":""},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"import fixtures"},{"line_number":7,"context_line":"import mock"},{"line_number":8,"context_line":"from testtools import matchers"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAM3%2F%2Bp0U%3D","line":5,"updated":"2013-02-22 18:30:24.000000000","message":"Hmmm…drop this extra blank line; there really should only be a single blank line between groups of imports.  Sorry I missed it in the previous changeset…","commit_id":"910fb63811619bd429da1d4a335030bc3e5754c5"},{"author":{"_account_id":6724,"name":"Vasyl Khomenko","email":"vasiliyk@yahoo-inc.com","username":"vkhomenko"},"change_message_id":"3d42da0e74962c781322238f441d7b07c1cb82e4","unresolved":false,"context_lines":[{"line_number":2,"context_line":"import re"},{"line_number":3,"context_line":"import sys"},{"line_number":4,"context_line":""},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"import fixtures"},{"line_number":7,"context_line":"import mock"},{"line_number":8,"context_line":"from testtools import matchers"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAM3%2F%2Bpvs%3D","line":5,"in_reply_to":"AAAAM3%2F%2Bp0U%3D","updated":"2013-02-22 18:58:35.000000000","message":"Done","commit_id":"910fb63811619bd429da1d4a335030bc3e5754c5"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"befdb0e7faf6983f913baf29672be31b90913efa","unresolved":false,"context_lines":[{"line_number":144,"context_line":"            self.fail(\u0027CommandError not raised\u0027)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    def test_password(self):"},{"line_number":147,"context_line":"        self.make_env(\u0027OS_PASSWORD\u0027)"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        @mock.patch(\u0027sys.stdin\u0027, side_effect\u003dmock.MagicMock)"},{"line_number":150,"context_line":"        @mock.patch(\u0027getpass.getpass\u0027, return_value\u003d\u0027password\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAM3%2F%2Bpzc%3D","line":147,"updated":"2013-02-22 18:30:24.000000000","message":"Now that I look at this, I find myself a little confused; it almost seems like we\u0027re creating an environment containing _only_ OS_PASSWORD.  Since we\u0027re only talking about excluding a single variable, how about changing make_env() to something more like:\n\n    def make_env(self, exclude\u003dNone):\n        for var, val in FAKE_ENV.items():\n            if var \u003d\u003d exclude:\n                continue\n            self.useFixture(fixtures.EnvironmentVariable(var, val))\n\nThen, using it, you can do:\n\n    self.make_env(exclude\u003d\u0027OS_PASSWORD\u0027)\n\nThis makes it a little more clear that we\u0027re not creating an environment with _only_ OS_PASSWORD, but rather that we\u0027re creating an environment _excluding_ OS_PASSWORD…","commit_id":"910fb63811619bd429da1d4a335030bc3e5754c5"},{"author":{"_account_id":6724,"name":"Vasyl Khomenko","email":"vasiliyk@yahoo-inc.com","username":"vkhomenko"},"change_message_id":"3d42da0e74962c781322238f441d7b07c1cb82e4","unresolved":false,"context_lines":[{"line_number":144,"context_line":"            self.fail(\u0027CommandError not raised\u0027)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    def test_password(self):"},{"line_number":147,"context_line":"        self.make_env(\u0027OS_PASSWORD\u0027)"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        @mock.patch(\u0027sys.stdin\u0027, side_effect\u003dmock.MagicMock)"},{"line_number":150,"context_line":"        @mock.patch(\u0027getpass.getpass\u0027, return_value\u003d\u0027password\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAM3%2F%2Bpvk%3D","line":147,"in_reply_to":"AAAAM3%2F%2Bpzc%3D","updated":"2013-02-22 18:58:35.000000000","message":"Done","commit_id":"910fb63811619bd429da1d4a335030bc3e5754c5"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"befdb0e7faf6983f913baf29672be31b90913efa","unresolved":false,"context_lines":[{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        @mock.patch(\u0027sys.stdin\u0027, side_effect\u003dmock.MagicMock)"},{"line_number":150,"context_line":"        @mock.patch(\u0027getpass.getpass\u0027, return_value\u003d\u0027password\u0027)"},{"line_number":151,"context_line":"        def test_password(*args, **kwargs):"},{"line_number":152,"context_line":"            stdout, stderr \u003d self.shell(\u0027list\u0027)"},{"line_number":153,"context_line":"            self.assertEqual((stdout + stderr), \u0027\\n\u0027)"},{"line_number":154,"context_line":"        test_password()"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAM3%2F%2Bpz8%3D","line":151,"updated":"2013-02-22 18:30:24.000000000","message":"Er, no, this wasn\u0027t what I was driving at.  My suggestion would reformat this entire test method as:\n\n    @mock.patch(\u0027sys.stdin\u0027, side_effect\u003dmock.MagicMock)\n    @mock.patch(\u0027getpass.getpass\u0027, return_value\u003d\u0027password\u0027)\n    def test_password(self, mock_getpass, mock_stdin):\n        self.make_env(\u0027OS_PASSWORD\u0027)\n\n        stdout, stderr \u003d self.shell(\u0027list\u0027)\n        self.assertEqual((stdout + stderr), \u0027\\n\u0027)\n\nThen there\u0027s no need for an inner function…","commit_id":"910fb63811619bd429da1d4a335030bc3e5754c5"},{"author":{"_account_id":6724,"name":"Vasyl Khomenko","email":"vasiliyk@yahoo-inc.com","username":"vkhomenko"},"change_message_id":"3d42da0e74962c781322238f441d7b07c1cb82e4","unresolved":false,"context_lines":[{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        @mock.patch(\u0027sys.stdin\u0027, side_effect\u003dmock.MagicMock)"},{"line_number":150,"context_line":"        @mock.patch(\u0027getpass.getpass\u0027, return_value\u003d\u0027password\u0027)"},{"line_number":151,"context_line":"        def test_password(*args, **kwargs):"},{"line_number":152,"context_line":"            stdout, stderr \u003d self.shell(\u0027list\u0027)"},{"line_number":153,"context_line":"            self.assertEqual((stdout + stderr), \u0027\\n\u0027)"},{"line_number":154,"context_line":"        test_password()"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAM3%2F%2BpwE%3D","line":151,"in_reply_to":"AAAAM3%2F%2Bpz8%3D","updated":"2013-02-22 18:58:35.000000000","message":"Done","commit_id":"910fb63811619bd429da1d4a335030bc3e5754c5"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"756841673a97e2c3c5c3c252024381ab15f59a61","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"    def make_env(self, exclude\u003dNone):"},{"line_number":23,"context_line":"        for var, val in FAKE_ENV.items():"},{"line_number":24,"context_line":"            if var in exclude:"},{"line_number":25,"context_line":"                continue"},{"line_number":26,"context_line":"            self.useFixture(fixtures.EnvironmentVariable(var, val))"},{"line_number":27,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2BppQ%3D","line":24,"updated":"2013-02-22 19:38:22.000000000","message":"You want \u0027\u003d\u003d\u0027 here…","commit_id":"42bb0c0b9cb495e8aef5579dd0e18f07a8d19853"},{"author":{"_account_id":6724,"name":"Vasyl Khomenko","email":"vasiliyk@yahoo-inc.com","username":"vkhomenko"},"change_message_id":"36d6836766ca5f8dd0a7009e59cada7a34134d95","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"    def make_env(self, exclude\u003dNone):"},{"line_number":23,"context_line":"        for var, val in FAKE_ENV.items():"},{"line_number":24,"context_line":"            if var in exclude:"},{"line_number":25,"context_line":"                continue"},{"line_number":26,"context_line":"            self.useFixture(fixtures.EnvironmentVariable(var, val))"},{"line_number":27,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2BpEw%3D","line":24,"in_reply_to":"AAAAM3%2F%2BppQ%3D","updated":"2013-02-23 12:19:00.000000000","message":"Done","commit_id":"42bb0c0b9cb495e8aef5579dd0e18f07a8d19853"}]}
