)]}'
{"cinder/tests/test_ibm_flashsystem.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"f9d7771f86e97c4f7057a0f8ae6aa944f5606860","unresolved":false,"context_lines":[{"line_number":645,"context_line":"        func \u003d getattr(self, \u0027_cmd_\u0027 + command)"},{"line_number":646,"context_line":"        out, err \u003d func(**kwargs)"},{"line_number":647,"context_line":""},{"line_number":648,"context_line":"        if check_exit_code and err:"},{"line_number":649,"context_line":"            raise processutils.ProcessExecutionError(exit_code\u003d1,"},{"line_number":650,"context_line":"                                                     stdout\u003dout,"},{"line_number":651,"context_line":"                                                     stderr\u003derr,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a930d6b_c0468b7a","line":648,"updated":"2015-01-27 17:18:05.000000000","message":"This isn\u0027t an equivalent test.  To match the functionality from before, you\u0027d also have to assert that \"err\" is a string.","commit_id":"cc6cdb4cd18fc9a6a16ec97f7f824256a6409a1f"},{"author":{"_account_id":7219,"name":"git-harry","email":"git-harry@live.co.uk","username":"git-harry"},"change_message_id":"858e10293dd3d98ed7fdac06922a945fffd82504","unresolved":false,"context_lines":[{"line_number":645,"context_line":"        func \u003d getattr(self, \u0027_cmd_\u0027 + command)"},{"line_number":646,"context_line":"        out, err \u003d func(**kwargs)"},{"line_number":647,"context_line":""},{"line_number":648,"context_line":"        if check_exit_code and err:"},{"line_number":649,"context_line":"            raise processutils.ProcessExecutionError(exit_code\u003d1,"},{"line_number":650,"context_line":"                                                     stdout\u003dout,"},{"line_number":651,"context_line":"                                                     stderr\u003derr,"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa81d914_44360205","line":648,"in_reply_to":"1a930d6b_c0468b7a","updated":"2015-01-27 21:38:41.000000000","message":"That\u0027s not quite correct. I\u0027d have to assert that it was something with a __len__ method which means list, set, tuple, dict etc. I don\u0027t know what the benefit of doing that would be. If bool(err) \u003d\u003d True and hasattr(err, \u0027__len__\u0027) \u003d\u003d True I don\u0027t see how that helps beyond just knowing bool(err) \u003d\u003d True.","commit_id":"cc6cdb4cd18fc9a6a16ec97f7f824256a6409a1f"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"70ed0fd5ca563fea2be94a68a4e078bbbf1bf577","unresolved":false,"context_lines":[{"line_number":645,"context_line":"        func \u003d getattr(self, \u0027_cmd_\u0027 + command)"},{"line_number":646,"context_line":"        out, err \u003d func(**kwargs)"},{"line_number":647,"context_line":""},{"line_number":648,"context_line":"        if check_exit_code and err:"},{"line_number":649,"context_line":"            raise processutils.ProcessExecutionError(exit_code\u003d1,"},{"line_number":650,"context_line":"                                                     stdout\u003dout,"},{"line_number":651,"context_line":"                                                     stderr\u003derr,"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa81d914_fc3c87e4","line":648,"in_reply_to":"fa81d914_44360205","updated":"2015-01-29 17:03:15.000000000","message":"Ok, but the fact that this now only asserts that bool(err) \u003d\u003d True instead of len(err) !\u003d 0, still seems to mean that it is allowing more things through than it did before.\n\nMaybe this is ok in this particular instance as far as benefit (I\u0027m not sure), but it really makes me question the premise that converting this idiom wholesale to another form is really a beneficial thing to do, when they aren\u0027t 100% equivalent.","commit_id":"cc6cdb4cd18fc9a6a16ec97f7f824256a6409a1f"},{"author":{"_account_id":7219,"name":"git-harry","email":"git-harry@live.co.uk","username":"git-harry"},"change_message_id":"fd3e1f4b453284a9905751f94c15a117cffeaf03","unresolved":false,"context_lines":[{"line_number":645,"context_line":"        func \u003d getattr(self, \u0027_cmd_\u0027 + command)"},{"line_number":646,"context_line":"        out, err \u003d func(**kwargs)"},{"line_number":647,"context_line":""},{"line_number":648,"context_line":"        if check_exit_code and err:"},{"line_number":649,"context_line":"            raise processutils.ProcessExecutionError(exit_code\u003d1,"},{"line_number":650,"context_line":"                                                     stdout\u003dout,"},{"line_number":651,"context_line":"                                                     stderr\u003derr,"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa81d914_dc3c18e5","line":648,"in_reply_to":"fa81d914_fc3c87e4","updated":"2015-01-29 21:41:04.000000000","message":"I think looking at this case is a good example of the general principle. With regard to this case, there may be the potential for it to allow more types through but, I don\u0027t necessarily believe that is a negative. I think we need to decide what the author was attempting to do with that line and what makes the most sense to do with that line.\n\nI suspect that the author of this line was simply trying to check that err is not an empty string based on the assumption that err is a string. Now I think that it is reasonable to assume that err is a string and if we do so the suggestion I have made makes perfect sense.\n\nIf it cannot be assumed that err is a string but the code wants to make sure it is, len seems a rather strange way to do it. Using len means that it is acceptable for err to be a set or dict but not int - that doesn\u0027t make much sense to me.\n\nWith the change I\u0027m suggesting if bool(err) \u003d\u003d True, ProcessExecutionError is going to try and use err to construct an error message for itself, if when this is attempted str(err) throws an exception it seems perfectly reasonable for it to happen there because err should be a string and if it isn\u0027t something is broken.","commit_id":"cc6cdb4cd18fc9a6a16ec97f7f824256a6409a1f"}],"cinder/tests/test_storwize_svc.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"f9d7771f86e97c4f7057a0f8ae6aa944f5606860","unresolved":false,"context_lines":[{"line_number":1460,"context_line":"        func \u003d getattr(self, \u0027_cmd_\u0027 + command)"},{"line_number":1461,"context_line":"        out, err \u003d func(**kwargs)"},{"line_number":1462,"context_line":""},{"line_number":1463,"context_line":"        if check_exit_code and err:"},{"line_number":1464,"context_line":"            raise processutils.ProcessExecutionError(exit_code\u003d1,"},{"line_number":1465,"context_line":"                                                     stdout\u003dout,"},{"line_number":1466,"context_line":"                                                     stderr\u003derr,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a930d6b_a31fc959","line":1463,"updated":"2015-01-27 17:18:05.000000000","message":"Same as before... this previously would fail if err was not a string, now it doesn\u0027t.","commit_id":"cc6cdb4cd18fc9a6a16ec97f7f824256a6409a1f"},{"author":{"_account_id":7219,"name":"git-harry","email":"git-harry@live.co.uk","username":"git-harry"},"change_message_id":"858e10293dd3d98ed7fdac06922a945fffd82504","unresolved":false,"context_lines":[{"line_number":1460,"context_line":"        func \u003d getattr(self, \u0027_cmd_\u0027 + command)"},{"line_number":1461,"context_line":"        out, err \u003d func(**kwargs)"},{"line_number":1462,"context_line":""},{"line_number":1463,"context_line":"        if check_exit_code and err:"},{"line_number":1464,"context_line":"            raise processutils.ProcessExecutionError(exit_code\u003d1,"},{"line_number":1465,"context_line":"                                                     stdout\u003dout,"},{"line_number":1466,"context_line":"                                                     stderr\u003derr,"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa81d914_7fcf13b1","line":1463,"in_reply_to":"1a930d6b_a31fc959","updated":"2015-01-27 21:38:41.000000000","message":"Following on from my reply to your other comment, I don\u0027t see that as an issue. err needs to be representable as a string it doesn\u0027t need to actually be a string. If the code wants to be 100% sure that err is a string then I would expect to see something like isinstance(err, str) or str(err) somewhere in this section of code.","commit_id":"cc6cdb4cd18fc9a6a16ec97f7f824256a6409a1f"}]}