)]}'
{"cinder/tests/unit/volume/drivers/test_zfssa.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0bccca4e95fe5ba0f48c7c15868afe4f0bb6bfb2","unresolved":false,"context_lines":[{"line_number":911,"context_line":"        self.configuration.zfssa_manage_policy \u003d \u0027strict\u0027"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"    def test_setup_nfs_client(self):"},{"line_number":914,"context_line":"        mock_execute \u003d self.mock_object(self.drv, \u0027_execute\u0027)"},{"line_number":915,"context_line":"        mock_execute.side_effect \u003d OSError("},{"line_number":916,"context_line":"            errno.ENOENT, \u0027No such file or directory.\u0027)"},{"line_number":917,"context_line":""},{"line_number":918,"context_line":"        with self.assertRaisesRegex(exception.NfsException,"},{"line_number":919,"context_line":"                                    \u0027/usr/sbin/mount is not installed\u0027):"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa6399be_b301da3a","line":916,"range":{"start_line":914,"start_character":0,"end_line":916,"end_character":55},"updated":"2016-10-13 10:16:26.000000000","message":"nit: Why not do this in one call?\n\n mock_execute \u003d self.mock_object(self.drv, \u0027_execute\u0027,\n                                 side_effect \u003d OSError(errno.ENOENT, \u0027Error\u0027))","commit_id":"955d7d15c4c795cb81a3ab2aa409a8fda4756870"},{"author":{"_account_id":13059,"name":"Kedar Vidvans","email":"kedar.vidvans@oracle.com","username":"kedar-vidvans"},"change_message_id":"989624904099af637948504293b00d4a6c203fc0","unresolved":false,"context_lines":[{"line_number":911,"context_line":"        self.configuration.zfssa_manage_policy \u003d \u0027strict\u0027"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"    def test_setup_nfs_client(self):"},{"line_number":914,"context_line":"        mock_execute \u003d self.mock_object(self.drv, \u0027_execute\u0027)"},{"line_number":915,"context_line":"        mock_execute.side_effect \u003d OSError("},{"line_number":916,"context_line":"            errno.ENOENT, \u0027No such file or directory.\u0027)"},{"line_number":917,"context_line":""},{"line_number":918,"context_line":"        with self.assertRaisesRegex(exception.NfsException,"},{"line_number":919,"context_line":"                                    \u0027/usr/sbin/mount is not installed\u0027):"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa6399be_07194e22","line":916,"range":{"start_line":914,"start_character":0,"end_line":916,"end_character":55},"in_reply_to":"fa6399be_b301da3a","updated":"2016-10-13 15:35:37.000000000","message":"Done","commit_id":"955d7d15c4c795cb81a3ab2aa409a8fda4756870"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0bccca4e95fe5ba0f48c7c15868afe4f0bb6bfb2","unresolved":false,"context_lines":[{"line_number":915,"context_line":"        mock_execute.side_effect \u003d OSError("},{"line_number":916,"context_line":"            errno.ENOENT, \u0027No such file or directory.\u0027)"},{"line_number":917,"context_line":""},{"line_number":918,"context_line":"        with self.assertRaisesRegex(exception.NfsException,"},{"line_number":919,"context_line":"                                    \u0027/usr/sbin/mount is not installed\u0027):"},{"line_number":920,"context_line":"            self.drv.do_setup(self.context)"},{"line_number":921,"context_line":"        mock_execute.assert_has_calls("},{"line_number":922,"context_line":"            [mock.call(\u0027mount.nfs\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa6399be_536b1ef1","line":919,"range":{"start_line":918,"start_character":0,"end_line":919,"end_character":72},"updated":"2016-10-13 10:16:26.000000000","message":"nit: We usually try not to test the contents of the error messages","commit_id":"955d7d15c4c795cb81a3ab2aa409a8fda4756870"},{"author":{"_account_id":13059,"name":"Kedar Vidvans","email":"kedar.vidvans@oracle.com","username":"kedar-vidvans"},"change_message_id":"989624904099af637948504293b00d4a6c203fc0","unresolved":false,"context_lines":[{"line_number":915,"context_line":"        mock_execute.side_effect \u003d OSError("},{"line_number":916,"context_line":"            errno.ENOENT, \u0027No such file or directory.\u0027)"},{"line_number":917,"context_line":""},{"line_number":918,"context_line":"        with self.assertRaisesRegex(exception.NfsException,"},{"line_number":919,"context_line":"                                    \u0027/usr/sbin/mount is not installed\u0027):"},{"line_number":920,"context_line":"            self.drv.do_setup(self.context)"},{"line_number":921,"context_line":"        mock_execute.assert_has_calls("},{"line_number":922,"context_line":"            [mock.call(\u0027mount.nfs\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa6399be_271652f1","line":919,"range":{"start_line":918,"start_character":0,"end_line":919,"end_character":72},"in_reply_to":"fa6399be_536b1ef1","updated":"2016-10-13 15:35:37.000000000","message":"Done","commit_id":"955d7d15c4c795cb81a3ab2aa409a8fda4756870"}],"cinder/volume/drivers/zfssa/zfssanfs.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"670d596868a7ee1aa2ffafa6a54cc076b1d13fc2","unresolved":false,"context_lines":[{"line_number":110,"context_line":"            LOG.error(msg)"},{"line_number":111,"context_line":"            raise exception.NfsException(msg)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        if platform.system() !\u003d \"SunOS\":"},{"line_number":114,"context_line":"            package \u003d \u0027mount.nfs\u0027"},{"line_number":115,"context_line":"            try:"},{"line_number":116,"context_line":"                self._execute(package, check_exit_code\u003dFalse, run_as_root\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3aaa91ec_76ecfdf3","line":113,"updated":"2016-07-01 15:48:49.000000000","message":"We should not add platform checks inside of volume drivers.\n\nI\u0027d prefer either, in order:\n\na) If the check for mount.nfs fails, run a similar check for a command that would be present on Solaris.\n\nb) Add a config option that allows this check to be skipped","commit_id":"8343721c06064a0dbccf6b2e2c40fbb2b424f7c9"},{"author":{"_account_id":12112,"name":"Tom Swanson","email":"tom_swanson@dell.com","username":"tomswanson"},"change_message_id":"fc7474876498164bad859537747266d47f493e21","unresolved":false,"context_lines":[{"line_number":110,"context_line":"            LOG.error(msg)"},{"line_number":111,"context_line":"            raise exception.NfsException(msg)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        if platform.system() !\u003d \"SunOS\":"},{"line_number":114,"context_line":"            package \u003d \u0027mount.nfs\u0027"},{"line_number":115,"context_line":"            try:"},{"line_number":116,"context_line":"                self._execute(package, check_exit_code\u003dFalse, run_as_root\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3aaa91ec_168139c4","line":113,"in_reply_to":"3aaa91ec_76ecfdf3","updated":"2016-07-01 15:54:42.000000000","message":"Both of these seem like good options but why the ban on platform checks?","commit_id":"8343721c06064a0dbccf6b2e2c40fbb2b424f7c9"},{"author":{"_account_id":13059,"name":"Kedar Vidvans","email":"kedar.vidvans@oracle.com","username":"kedar-vidvans"},"change_message_id":"9555ef537dba6c3a66ac9ffbdd9d05994045db4d","unresolved":false,"context_lines":[{"line_number":110,"context_line":"            LOG.error(msg)"},{"line_number":111,"context_line":"            raise exception.NfsException(msg)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        if platform.system() !\u003d \"SunOS\":"},{"line_number":114,"context_line":"            package \u003d \u0027mount.nfs\u0027"},{"line_number":115,"context_line":"            try:"},{"line_number":116,"context_line":"                self._execute(package, check_exit_code\u003dFalse, run_as_root\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3aaa91ec_78722613","line":113,"in_reply_to":"3aaa91ec_76ecfdf3","updated":"2016-07-01 18:13:31.000000000","message":"I chose option a.","commit_id":"8343721c06064a0dbccf6b2e2c40fbb2b424f7c9"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"2e0a33c28163395507fdcfc6636e3cbc4adfe950","unresolved":false,"context_lines":[{"line_number":118,"context_line":"            self._execute(package, check_exit_code\u003dFalse, run_as_root\u003dTrue)"},{"line_number":119,"context_line":"        except OSError as exc:"},{"line_number":120,"context_line":"            if exc.errno \u003d\u003d errno.ENOENT:"},{"line_number":121,"context_line":"                msg \u003d _(\u0027%s is not installed. Trying with \u0027"},{"line_number":122,"context_line":"                        \u0027/usr/sbin/mount.\u0027) % package"},{"line_number":123,"context_line":"                LOG.error(msg)"},{"line_number":124,"context_line":"                package \u003d \u0027/usr/sbin/mount\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa6399be_ea8805ea","line":121,"range":{"start_line":121,"start_character":16,"end_line":121,"end_character":19},"updated":"2016-10-11 18:30:25.000000000","message":"msg at this point is only used in the LOG.error. Please change to _LE() and move this into the LOG.error call, switching % to , to delay string formatting.","commit_id":"560842bab02732d99f0b66bec00ba5f19d9e75b6"},{"author":{"_account_id":13059,"name":"Kedar Vidvans","email":"kedar.vidvans@oracle.com","username":"kedar-vidvans"},"change_message_id":"83df5cf144e6f7d4618f08d7220335b0f8ae0100","unresolved":false,"context_lines":[{"line_number":118,"context_line":"            self._execute(package, check_exit_code\u003dFalse, run_as_root\u003dTrue)"},{"line_number":119,"context_line":"        except OSError as exc:"},{"line_number":120,"context_line":"            if exc.errno \u003d\u003d errno.ENOENT:"},{"line_number":121,"context_line":"                msg \u003d _(\u0027%s is not installed. Trying with \u0027"},{"line_number":122,"context_line":"                        \u0027/usr/sbin/mount.\u0027) % package"},{"line_number":123,"context_line":"                LOG.error(msg)"},{"line_number":124,"context_line":"                package \u003d \u0027/usr/sbin/mount\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa6399be_18f753d4","line":121,"range":{"start_line":121,"start_character":16,"end_line":121,"end_character":19},"in_reply_to":"fa6399be_ea8805ea","updated":"2016-10-11 19:09:58.000000000","message":"Done","commit_id":"560842bab02732d99f0b66bec00ba5f19d9e75b6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0bccca4e95fe5ba0f48c7c15868afe4f0bb6bfb2","unresolved":false,"context_lines":[{"line_number":113,"context_line":"            LOG.error(msg)"},{"line_number":114,"context_line":"            raise exception.NfsException(msg)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        package \u003d \u0027mount.nfs\u0027"},{"line_number":117,"context_line":"        try:"},{"line_number":118,"context_line":"            self._execute(package, check_exit_code\u003dFalse, run_as_root\u003dTrue)"},{"line_number":119,"context_line":"        except OSError as exc:"},{"line_number":120,"context_line":"            if exc.errno \u003d\u003d errno.ENOENT:"},{"line_number":121,"context_line":"                LOG.error(_LE(\u0027%s is not installed. Trying with \u0027"},{"line_number":122,"context_line":"                              \u0027/usr/sbin/mount.\u0027), package)"},{"line_number":123,"context_line":"                package \u003d \u0027/usr/sbin/mount\u0027"},{"line_number":124,"context_line":"                try:"},{"line_number":125,"context_line":"                    self._execute(package, check_exit_code\u003dFalse,"},{"line_number":126,"context_line":"                                  run_as_root\u003dTrue)"},{"line_number":127,"context_line":"                except OSError as exc:"},{"line_number":128,"context_line":"                    if exc.errno \u003d\u003d errno.ENOENT:"},{"line_number":129,"context_line":"                        msg \u003d _(\u0027%s is not installed\u0027) % package"},{"line_number":130,"context_line":"                        raise exception.NfsException(msg)"},{"line_number":131,"context_line":"                    else:"},{"line_number":132,"context_line":"                        raise"},{"line_number":133,"context_line":"            else:"},{"line_number":134,"context_line":"                raise"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        lcfg \u003d self.configuration"},{"line_number":137,"context_line":"        LOG.info(_LI(\u0027Connecting to host: %s.\u0027), lcfg.san_ip)"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa6399be_53aebe81","line":134,"range":{"start_line":116,"start_character":0,"end_line":134,"end_character":21},"updated":"2016-10-13 10:16:26.000000000","message":"-1: I think we should simplify this\n\n packages \u003d (\u0027mount.nfs\u0027, \u0027/usr/sbin/mount\u0027)\n for package in packages:\n     try:\n         self._execute(package, check_exit_code\u003dFalse, run_as_root\u003dTrue)\n     except OSError as exc:\n         if exc.errno !\u003d errno.ENOENT:\n             raise\n         LOG.error(_LE(\u0027%s is not installed.\u0027), package)\n     else:\n         msg \u003d utils.build_or_str(packages, \u0027%s needs to be installed.\u0027)\n         raise exception.NfsException(msg)","commit_id":"955d7d15c4c795cb81a3ab2aa409a8fda4756870"},{"author":{"_account_id":13059,"name":"Kedar Vidvans","email":"kedar.vidvans@oracle.com","username":"kedar-vidvans"},"change_message_id":"989624904099af637948504293b00d4a6c203fc0","unresolved":false,"context_lines":[{"line_number":113,"context_line":"            LOG.error(msg)"},{"line_number":114,"context_line":"            raise exception.NfsException(msg)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        package \u003d \u0027mount.nfs\u0027"},{"line_number":117,"context_line":"        try:"},{"line_number":118,"context_line":"            self._execute(package, check_exit_code\u003dFalse, run_as_root\u003dTrue)"},{"line_number":119,"context_line":"        except OSError as exc:"},{"line_number":120,"context_line":"            if exc.errno \u003d\u003d errno.ENOENT:"},{"line_number":121,"context_line":"                LOG.error(_LE(\u0027%s is not installed. Trying with \u0027"},{"line_number":122,"context_line":"                              \u0027/usr/sbin/mount.\u0027), package)"},{"line_number":123,"context_line":"                package \u003d \u0027/usr/sbin/mount\u0027"},{"line_number":124,"context_line":"                try:"},{"line_number":125,"context_line":"                    self._execute(package, check_exit_code\u003dFalse,"},{"line_number":126,"context_line":"                                  run_as_root\u003dTrue)"},{"line_number":127,"context_line":"                except OSError as exc:"},{"line_number":128,"context_line":"                    if exc.errno \u003d\u003d errno.ENOENT:"},{"line_number":129,"context_line":"                        msg \u003d _(\u0027%s is not installed\u0027) % package"},{"line_number":130,"context_line":"                        raise exception.NfsException(msg)"},{"line_number":131,"context_line":"                    else:"},{"line_number":132,"context_line":"                        raise"},{"line_number":133,"context_line":"            else:"},{"line_number":134,"context_line":"                raise"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        lcfg \u003d self.configuration"},{"line_number":137,"context_line":"        LOG.info(_LI(\u0027Connecting to host: %s.\u0027), lcfg.san_ip)"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa6399be_b25aa228","line":134,"range":{"start_line":116,"start_character":0,"end_line":134,"end_character":21},"in_reply_to":"fa6399be_53aebe81","updated":"2016-10-13 15:35:37.000000000","message":"I dont want the \"else\" condition in the try-except-else to run when the _execute has run successfully without exception whereas in the above it will. see: https://docs.python.org/2.7/tutorial/errors.html#handling-exceptions\n\nThe code I wrote also ensures that no exception is thrown when mount.nfs is run but not found.","commit_id":"955d7d15c4c795cb81a3ab2aa409a8fda4756870"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"18054633049a31313f1c28fa6a59539c0635ca76","unresolved":false,"context_lines":[{"line_number":113,"context_line":"            LOG.error(msg)"},{"line_number":114,"context_line":"            raise exception.NfsException(msg)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        package \u003d \u0027mount.nfs\u0027"},{"line_number":117,"context_line":"        try:"},{"line_number":118,"context_line":"            self._execute(package, check_exit_code\u003dFalse, run_as_root\u003dTrue)"},{"line_number":119,"context_line":"        except OSError as exc:"},{"line_number":120,"context_line":"            if exc.errno \u003d\u003d errno.ENOENT:"},{"line_number":121,"context_line":"                LOG.error(_LE(\u0027%s is not installed. Trying with \u0027"},{"line_number":122,"context_line":"                              \u0027/usr/sbin/mount.\u0027), package)"},{"line_number":123,"context_line":"                package \u003d \u0027/usr/sbin/mount\u0027"},{"line_number":124,"context_line":"                try:"},{"line_number":125,"context_line":"                    self._execute(package, check_exit_code\u003dFalse,"},{"line_number":126,"context_line":"                                  run_as_root\u003dTrue)"},{"line_number":127,"context_line":"                except OSError as exc:"},{"line_number":128,"context_line":"                    if exc.errno \u003d\u003d errno.ENOENT:"},{"line_number":129,"context_line":"                        msg \u003d _(\u0027%s is not installed\u0027) % package"},{"line_number":130,"context_line":"                        raise exception.NfsException(msg)"},{"line_number":131,"context_line":"                    else:"},{"line_number":132,"context_line":"                        raise"},{"line_number":133,"context_line":"            else:"},{"line_number":134,"context_line":"                raise"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        lcfg \u003d self.configuration"},{"line_number":137,"context_line":"        LOG.info(_LI(\u0027Connecting to host: %s.\u0027), lcfg.san_ip)"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa6399be_efefac1b","line":134,"range":{"start_line":116,"start_character":0,"end_line":134,"end_character":21},"in_reply_to":"fa6399be_b25aa228","updated":"2016-10-14 11:06:42.000000000","message":"Oh, good catch, that\u0027s because I forgot to write the \"break\" after the self._execute.   :-)\n\nI think my proposal is more readable, has less repetition, and is easier to extend to cover new cases.\n\nBut it\u0027s your driver, so I won\u0027t downvote if you feel strongly about your solution, but I won\u0027t +2 either.","commit_id":"955d7d15c4c795cb81a3ab2aa409a8fda4756870"},{"author":{"_account_id":13059,"name":"Kedar Vidvans","email":"kedar.vidvans@oracle.com","username":"kedar-vidvans"},"change_message_id":"ce9d3987d66dbd575046c88a0f09159bcf99aba2","unresolved":false,"context_lines":[{"line_number":113,"context_line":"            LOG.error(msg)"},{"line_number":114,"context_line":"            raise exception.NfsException(msg)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        package \u003d \u0027mount.nfs\u0027"},{"line_number":117,"context_line":"        try:"},{"line_number":118,"context_line":"            self._execute(package, check_exit_code\u003dFalse, run_as_root\u003dTrue)"},{"line_number":119,"context_line":"        except OSError as exc:"},{"line_number":120,"context_line":"            if exc.errno \u003d\u003d errno.ENOENT:"},{"line_number":121,"context_line":"                LOG.error(_LE(\u0027%s is not installed. Trying with \u0027"},{"line_number":122,"context_line":"                              \u0027/usr/sbin/mount.\u0027), package)"},{"line_number":123,"context_line":"                package \u003d \u0027/usr/sbin/mount\u0027"},{"line_number":124,"context_line":"                try:"},{"line_number":125,"context_line":"                    self._execute(package, check_exit_code\u003dFalse,"},{"line_number":126,"context_line":"                                  run_as_root\u003dTrue)"},{"line_number":127,"context_line":"                except OSError as exc:"},{"line_number":128,"context_line":"                    if exc.errno \u003d\u003d errno.ENOENT:"},{"line_number":129,"context_line":"                        msg \u003d _(\u0027%s is not installed\u0027) % package"},{"line_number":130,"context_line":"                        raise exception.NfsException(msg)"},{"line_number":131,"context_line":"                    else:"},{"line_number":132,"context_line":"                        raise"},{"line_number":133,"context_line":"            else:"},{"line_number":134,"context_line":"                raise"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        lcfg \u003d self.configuration"},{"line_number":137,"context_line":"        LOG.info(_LI(\u0027Connecting to host: %s.\u0027), lcfg.san_ip)"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa6399be_a125c05c","line":134,"range":{"start_line":116,"start_character":0,"end_line":134,"end_character":21},"in_reply_to":"fa6399be_efefac1b","updated":"2016-10-14 14:45:20.000000000","message":"Gorka, thanks for the review. The problem in your proposal which makes me want to stick with mine is that when both the packages are not found and exceptions are thrown in both cases, no exception will be raised. Since this is the do_setup method, I would like an exception to be raised so that the driver is prevented from being loaded successfully.\n\nWe can however, modify your proposal in the exception block to check if this is that last package being verified and if it is then raise an exception. Then there is no need for the else block. I will add that in the next revision. Please check it out.","commit_id":"955d7d15c4c795cb81a3ab2aa409a8fda4756870"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"76cf4d0b542f70aeaf55bdf89eb84bc9534eb6b8","unresolved":false,"context_lines":[{"line_number":115,"context_line":"            self._execute(package, check_exit_code\u003dFalse, run_as_root\u003dTrue)"},{"line_number":116,"context_line":"        except OSError as exc:"},{"line_number":117,"context_line":"            if exc.errno \u003d\u003d errno.ENOENT:"},{"line_number":118,"context_line":"                msg \u003d _(\u0027%s is not installed. Trying with \u0027"},{"line_number":119,"context_line":"                        \u0027/usr/sbin/mount.\u0027) % package"},{"line_number":120,"context_line":"                LOG.error(msg)"},{"line_number":121,"context_line":"                package \u003d \u0027/usr/sbin/mount\u0027"},{"line_number":122,"context_line":"                try:"},{"line_number":123,"context_line":"                    self._execute(package, check_exit_code\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa6399be_ef316c5b","line":120,"range":{"start_line":118,"start_character":0,"end_line":120,"end_character":30},"updated":"2016-10-14 11:14:12.000000000","message":"-1: According to the guidelines we should be using delayed interpolation [1] like you were doing before:\n\n msg \u003d _(\u0027%s is not installed. Trying with \u0027\n         \u0027/usr/sbin/mount.\u0027)\n LOG.error(msg, package)\n\n\n[1]: http://docs.openstack.org/developer/oslo.i18n/guidelines.html#adding-variables-to-log-messages","commit_id":"2d414bdb8c20b628a8a72c58ed31061fecc77b25"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"7764186121d3271422d651261cd7bfad20501570","unresolved":false,"context_lines":[{"line_number":119,"context_line":"                if exc.errno !\u003d errno.ENOENT:"},{"line_number":120,"context_line":"                    raise"},{"line_number":121,"context_line":"                LOG.error(_LE(\u0027%s is not installed.\u0027), package)"},{"line_number":122,"context_line":"                if packages[-1] \u003d\u003d package:"},{"line_number":123,"context_line":"                    msg \u003d utils.build_or_str(packages, \u0027%s needs to be \u0027"},{"line_number":124,"context_line":"                                             \u0027installed.\u0027)"},{"line_number":125,"context_line":"                    raise exception.NfsException(msg)"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa6399be_c565321e","line":122,"updated":"2016-10-17 09:29:02.000000000","message":"nit: We could replace this with an else clause at the `for` level, that way if the for completes (didn\u0027t break on L117) it will raise the exception.  Sorry for mis-indenting it in my earlier comment.","commit_id":"a55057e1c57990f8d6402de61cf01e32063b2182"},{"author":{"_account_id":13059,"name":"Kedar Vidvans","email":"kedar.vidvans@oracle.com","username":"kedar-vidvans"},"change_message_id":"e297a49ecacafab0f28d3ad6dccc5fd5e5a01408","unresolved":false,"context_lines":[{"line_number":119,"context_line":"                if exc.errno !\u003d errno.ENOENT:"},{"line_number":120,"context_line":"                    raise"},{"line_number":121,"context_line":"                LOG.error(_LE(\u0027%s is not installed.\u0027), package)"},{"line_number":122,"context_line":"                if packages[-1] \u003d\u003d package:"},{"line_number":123,"context_line":"                    msg \u003d utils.build_or_str(packages, \u0027%s needs to be \u0027"},{"line_number":124,"context_line":"                                             \u0027installed.\u0027)"},{"line_number":125,"context_line":"                    raise exception.NfsException(msg)"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa6399be_cd10f05b","line":122,"in_reply_to":"fa6399be_c565321e","updated":"2016-10-17 13:38:15.000000000","message":"Gorka, I am not a fan of the else in \"for - else\" and \"try-except-else\" clauses, but I will do it this time;).","commit_id":"a55057e1c57990f8d6402de61cf01e32063b2182"}]}