)]}'
{"ironicclient/shell.py":[{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"ee330eb773475622cb1b1327b641a5dd439c0a8c","unresolved":false,"context_lines":[{"line_number":254,"context_line":"                # Check for Ctl-D"},{"line_number":255,"context_line":"                try:"},{"line_number":256,"context_line":"                    args.os_password \u003d getpass.getpass("},{"line_number":257,"context_line":"                        \u0027OS Password: \u0027)"},{"line_number":258,"context_line":"                except EOFError:"},{"line_number":259,"context_line":"                    pass"},{"line_number":260,"context_line":"            # No password because we didn\u0027t have a tty or the"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_ed280588","line":257,"updated":"2014-10-16 20:38:27.000000000","message":"I know the env variables prefix with OS, but I don\u0027t like this in a user-facing string. Is this what keystoneclient uses? I think it could confuse them; is it the Operating System password or OpenStack password? Not sure what would be the best substitute. Cloud account password? Tenant password?","commit_id":"d606d5dd6175d02ec0feec47de1282b0f9703bc4"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"4c9bb9b228d1689530b3832ad2261a917d4c5314","unresolved":false,"context_lines":[{"line_number":254,"context_line":"                # Check for Ctl-D"},{"line_number":255,"context_line":"                try:"},{"line_number":256,"context_line":"                    args.os_password \u003d getpass.getpass("},{"line_number":257,"context_line":"                        \u0027OS Password: \u0027)"},{"line_number":258,"context_line":"                except EOFError:"},{"line_number":259,"context_line":"                    pass"},{"line_number":260,"context_line":"            # No password because we didn\u0027t have a tty or the"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_93894c71","line":257,"in_reply_to":"baa201ad_1f956c32","updated":"2014-10-17 09:43:45.000000000","message":"I vote for OpenStack Password or something. As Davis said term \"OS\" is overloaded and some people in OpenStack expressed negative feelings towards using it.","commit_id":"d606d5dd6175d02ec0feec47de1282b0f9703bc4"},{"author":{"_account_id":12715,"name":"John Trowbridge","email":"trown@redhat.com","username":"trown"},"change_message_id":"401db4d13d84ef4b3d2d46a807b359305922bc60","unresolved":false,"context_lines":[{"line_number":254,"context_line":"                # Check for Ctl-D"},{"line_number":255,"context_line":"                try:"},{"line_number":256,"context_line":"                    args.os_password \u003d getpass.getpass("},{"line_number":257,"context_line":"                        \u0027OS Password: \u0027)"},{"line_number":258,"context_line":"                except EOFError:"},{"line_number":259,"context_line":"                    pass"},{"line_number":260,"context_line":"            # No password because we didn\u0027t have a tty or the"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_1f956c32","line":257,"in_reply_to":"baa201ad_ed280588","updated":"2014-10-16 21:44:44.000000000","message":"This is the string that is used in keystone, and now in neutron as well. I think it matches with the env variable (OS_PASSWORD) and the arg (--os-username). I think being consistent with these is less confusing than not being consistent.","commit_id":"d606d5dd6175d02ec0feec47de1282b0f9703bc4"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"823426dfc1220b3283324522653d744c0f702b88","unresolved":false,"context_lines":[{"line_number":254,"context_line":"                # Check for Ctl-D"},{"line_number":255,"context_line":"                try:"},{"line_number":256,"context_line":"                    args.os_password \u003d getpass.getpass("},{"line_number":257,"context_line":"                        \u0027OpenStack Password: \u0027)"},{"line_number":258,"context_line":"                except EOFError:"},{"line_number":259,"context_line":"                    pass"},{"line_number":260,"context_line":"            # No password because we didn\u0027t have a tty or the"}],"source_content_type":"text/x-python","patch_set":2,"id":"9aa7fdbe_0b9f9953","line":257,"updated":"2014-10-20 15:57:47.000000000","message":"I don\u0027t know if this is a nit or a stylistic thing, so I\u0027m just mentioning and you can decide. Instead of \u0027Password\u0027 -\u003e \u0027password\u0027.\n\nAnd for more points, I\u0027d actually like a longer message. Something like \u0027Please enter the OpenStack password for user \u003cusername\u003e:\u0027. (Hmm,unless there\u0027s some sort of security risk in displaying the username.)","commit_id":"46f9f99477ab810fcf7a022b35b40ff7ada9283a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"823426dfc1220b3283324522653d744c0f702b88","unresolved":false,"context_lines":[{"line_number":262,"context_line":"            if not args.os_password:"},{"line_number":263,"context_line":"                raise exc.CommandError(_(\"You must provide a password via \""},{"line_number":264,"context_line":"                                         \"either --os-password or via \""},{"line_number":265,"context_line":"                                         \"env[OS_PASSWORD]\"))"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"            if not (args.os_tenant_id or args.os_tenant_name):"},{"line_number":268,"context_line":"                raise exc.CommandError(_(\"You must provide a tenant_id via \""}],"source_content_type":"text/x-python","patch_set":2,"id":"9aa7fdbe_ab05adc3","line":265,"updated":"2014-10-20 15:57:47.000000000","message":"This needs to be updated. Something like \u0027You must provide a password when prompted, via --os-password, or via env[OS_PASSWORD]\u0027.","commit_id":"46f9f99477ab810fcf7a022b35b40ff7ada9283a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"4bf4bc44ddbde0b528377fc3f0881343d68f5ab4","unresolved":false,"context_lines":[{"line_number":248,"context_line":"                                         \"either --os-username or via \""},{"line_number":249,"context_line":"                                         \"env[OS_USERNAME]\"))"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        if not args.os_password:"},{"line_number":252,"context_line":"            # No password, If we\u0027ve got a tty, try prompting for it"},{"line_number":253,"context_line":"            if hasattr(sys.stdin, \u0027isatty\u0027) and sys.stdin.isatty():"},{"line_number":254,"context_line":"                # Check for Ctl-D"}],"source_content_type":"text/x-python","patch_set":4,"id":"9aa7fdbe_a7f782f4","line":251,"updated":"2014-10-21 13:34:25.000000000","message":"I just noticed this. This entire \u0027if\u0027 needs to be indented another level, otherwise, if the user specified auth_token \u0026 ironic_url, they will be prompted for a password, and they shouldn\u0027t be.","commit_id":"476a5e28b21d8ccb06a0aa604c90fc70ec56dc15"}],"ironicclient/tests/test_shell.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"4bf4bc44ddbde0b528377fc3f0881343d68f5ab4","unresolved":false,"context_lines":[{"line_number":110,"context_line":"            self.assertEqual(required, message.args)"},{"line_number":111,"context_line":"        else:"},{"line_number":112,"context_line":"            self.fail(\u0027CommandError not raised\u0027)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def test_bash_completion(self):"},{"line_number":115,"context_line":"        stdout \u003d self.shell(\u0027bash-completion\u0027)"},{"line_number":116,"context_line":"        # just check we have some output"}],"source_content_type":"text/x-python","patch_set":4,"id":"9aa7fdbe_e7918a9b","line":113,"updated":"2014-10-21 13:34:25.000000000","message":"I suggest checking that mock_getpass got called. Otherwise, I suspect this test would have passed w/o your code change, and you want to test your changes. (I suppose we could add another test where sys.stdin.isatty does\u0027t exist/is false, so that your code path isn\u0027t invoked.)","commit_id":"476a5e28b21d8ccb06a0aa604c90fc70ec56dc15"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"6674f632d5f1533468517ddb83316262277dc668","unresolved":false,"context_lines":[{"line_number":102,"context_line":"    def test_password_prompted(self, mock_getpass, mock_stdin):"},{"line_number":103,"context_line":"        self.make_env(exclude\u003d\u0027OS_PASSWORD\u0027)"},{"line_number":104,"context_line":"        # We will get an Authorization Failure because there is no keystone,"},{"line_number":105,"context_line":"        # but we are only interested in making sure getpass was called."},{"line_number":106,"context_line":"        try:"},{"line_number":107,"context_line":"            self.shell(\u0027node-list\u0027)"},{"line_number":108,"context_line":"        except:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_066e8796","line":105,"updated":"2014-10-21 19:08:07.000000000","message":"Hmm. I suspect ideally, we wouldn\u0027t want to even call the keystoneclient library in a test. But adding that is probably outside the scope of this patch. (There\u0027s also a patch that got merged that might have some of those hooks.)\n\nI think that maybe we could modify this:\n\n1. add at the top of this file: \u0027from keystoneclient import exceptions as keystone_exc\u0027\n\n2. instead of this try/except, do something like \u0027self.assertRaises(keystone_exc.AuthorizationFailure, self.shell, \u0027node-list\u0027)\n\nbecause we only expect this exception, not any other.","commit_id":"341d6e45a7f47ae2368c89445aa242eacb5127f3"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"6674f632d5f1533468517ddb83316262277dc668","unresolved":false,"context_lines":[{"line_number":107,"context_line":"            self.shell(\u0027node-list\u0027)"},{"line_number":108,"context_line":"        except:"},{"line_number":109,"context_line":"            pass"},{"line_number":110,"context_line":"        self.assertTrue(mock_getpass.called)"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    @mock.patch(\u0027sys.stdin\u0027, side_effect\u003dmock.MagicMock)"},{"line_number":113,"context_line":"    @mock.patch(\u0027getpass.getpass\u0027, side_effect\u003dEOFError)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_a6de9b3d","line":110,"updated":"2014-10-21 19:08:07.000000000","message":"I think it is clearer if you do something like \u0027mock_getpass.assert_called_with(\u0027OpenStack Password: \u0027)\n\nOr use mock.ANY instead of the \u0027OpenStack Passw...\u0027","commit_id":"341d6e45a7f47ae2368c89445aa242eacb5127f3"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"6674f632d5f1533468517ddb83316262277dc668","unresolved":false,"context_lines":[{"line_number":121,"context_line":"        except exc.CommandError as message:"},{"line_number":122,"context_line":"            self.assertEqual(required, message.args)"},{"line_number":123,"context_line":"        else:"},{"line_number":124,"context_line":"            self.fail(\u0027CommandError not raised\u0027)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    def test_bash_completion(self):"},{"line_number":127,"context_line":"        stdout \u003d self.shell(\u0027bash-completion\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_06ca076e","line":124,"updated":"2014-10-21 19:08:07.000000000","message":"This test will also end up calling getpass.getpass(). (It won\u0027t if you change the sys.stdin stuff I think.) If it calls getpass.getpass(), I think a mock_getpass.assert_called_with(...) might be good here, to make that clear.","commit_id":"341d6e45a7f47ae2368c89445aa242eacb5127f3"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"cba5e57b30bb4c0128217f07475a8359f8a3aefc","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        except exc.CommandError as message:"},{"line_number":143,"context_line":"            self.assertEqual(required, message.args)"},{"line_number":144,"context_line":"        else:"},{"line_number":145,"context_line":"            self.fail(\u0027CommandError not raised\u0027)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    def test_bash_completion(self):"},{"line_number":148,"context_line":"        stdout \u003d self.shell(\u0027bash-completion\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"9aa7fdbe_bb512eda","line":145,"updated":"2014-10-22 16:29:35.000000000","message":"I still think you should put in a mock_getpass.assert_called_with() here because getpass.getpass will get called. And this exception could happen regardless of getpass.getpass being called or not.\n\nThis test name might be misleading because there is a password prompt here, just that it is simulating a user doing ctrl-D when prompted. So maybe it is something like test_password_prompted_ctrlD()\n\nIf you don\u0027t mock sys.stdin, then getpass.getpass won\u0027t get called, and that would be a different test that could be test_no_password() or test_no_password_prompt()","commit_id":"b02683ea8d4c253f0725785840fe6a7ff492187a"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"ccf4980f51523d4073f11791333b14f3c316ede5","unresolved":false,"context_lines":[{"line_number":154,"context_line":"        except exc.CommandError as message:"},{"line_number":155,"context_line":"            self.assertEqual(required, message.args)"},{"line_number":156,"context_line":"        else:"},{"line_number":157,"context_line":"            self.fail(\u0027CommandError not raised\u0027)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    def test_bash_completion(self):"},{"line_number":160,"context_line":"        stdout \u003d self.shell(\u0027bash-completion\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9aa7fdbe_48c94358","line":157,"updated":"2014-10-23 10:47:09.000000000","message":"I wish we would not be using Python 2.6 and could just write:\n\n self.assertRaisesRegexp(exc.CommandError, required, self.shell, \u0027node-list\u0027)","commit_id":"9d6137380b09620c1e1039fa091f3e78b3c334b7"}]}