)]}'
{"cinder/brick/remotefs/remotefs.py":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"4143a69b1a2cd21ce9b90863149b7d6994545019","unresolved":false,"context_lines":[{"line_number":89,"context_line":"            return"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        self._execute(\u0027mkdir\u0027, \u0027-p\u0027, mount_path, check_exit_code\u003d0)"},{"line_number":92,"context_line":"        if self._mount_type \u003d\u003d \u0027nfs\u0027:"},{"line_number":93,"context_line":"            self._mount_nfs(share, mount_path, flags)"},{"line_number":94,"context_line":"        else:"},{"line_number":95,"context_line":"            self._do_mount(self._mount_type, share, mount_path,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAUX%2F%2Fd2w%3D","line":92,"updated":"2014-02-13 01:00:17.000000000","message":"I think this should be  if self._mount_type is \u0027nfs\u0027:","commit_id":"747b5496f5b843e552065bf6c51f9d7d08b6b70e"},{"author":{"_account_id":6094,"name":"Navneet Singh","email":"navsinghnitd@gmail.com","username":"singn"},"change_message_id":"3b629f0799e0cd944609a18b9f9b3b8628a0f4e9","unresolved":false,"context_lines":[{"line_number":89,"context_line":"            return"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        self._execute(\u0027mkdir\u0027, \u0027-p\u0027, mount_path, check_exit_code\u003d0)"},{"line_number":92,"context_line":"        if self._mount_type \u003d\u003d \u0027nfs\u0027:"},{"line_number":93,"context_line":"            self._mount_nfs(share, mount_path, flags)"},{"line_number":94,"context_line":"        else:"},{"line_number":95,"context_line":"            self._do_mount(self._mount_type, share, mount_path,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAUX%2F%2FcX8%3D","line":92,"in_reply_to":"AAAAUX%2F%2Fd2w%3D","updated":"2014-02-13 06:38:46.000000000","message":"I slightly defer on this one. In python \u0027is\u0027 used for identity comparison and \u0027\u003d\u003d\u0027 is used for comparing values. If x \u003d\u003d y does not imply x is y. I think its fine. In __init__ method also \u0027\u003d\u003d\u0027 method is used for comparison.","commit_id":"747b5496f5b843e552065bf6c51f9d7d08b6b70e"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"4143a69b1a2cd21ce9b90863149b7d6994545019","unresolved":false,"context_lines":[{"line_number":128,"context_line":"        raise exception.BrickException(_(\"NFS mount failed for share %(sh)s.\""},{"line_number":129,"context_line":"                                         \"Error - %(error)s\")"},{"line_number":130,"context_line":"                                       % {\u0027sh\u0027: nfs_share,"},{"line_number":131,"context_line":"                                          \u0027error\u0027: mnt_errors})"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"    def _check_nfs_options(self):"},{"line_number":134,"context_line":"        \"\"\"Checks and prepares nfs mount type options.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAUX%2F%2Fd1Y%3D","line":131,"updated":"2014-02-13 01:00:17.000000000","message":"This is confusing ... So, if none of the types work then you print an exception that includes all of the errors without any association to the type the error is for?  I think it would be better to do a LOG.error with the exception for each failure.  Make sure to use six.text_type(e) on the exception.  Then raise the exception at the end that none of the NFS types worked and reference the earlier logs for details.","commit_id":"747b5496f5b843e552065bf6c51f9d7d08b6b70e"},{"author":{"_account_id":6094,"name":"Navneet Singh","email":"navsinghnitd@gmail.com","username":"singn"},"change_message_id":"3b629f0799e0cd944609a18b9f9b3b8628a0f4e9","unresolved":false,"context_lines":[{"line_number":128,"context_line":"        raise exception.BrickException(_(\"NFS mount failed for share %(sh)s.\""},{"line_number":129,"context_line":"                                         \"Error - %(error)s\")"},{"line_number":130,"context_line":"                                       % {\u0027sh\u0027: nfs_share,"},{"line_number":131,"context_line":"                                          \u0027error\u0027: mnt_errors})"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"    def _check_nfs_options(self):"},{"line_number":134,"context_line":"        \"\"\"Checks and prepares nfs mount type options.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAUX%2F%2FcfE%3D","line":131,"in_reply_to":"AAAAUX%2F%2Fd1Y%3D","updated":"2014-02-13 06:38:46.000000000","message":"The errors are associated with mount type used, mnt_errors is a dictionary here. The exception is not printed as error because this feature addresses implicit retries for pnfs. As pnfs may not be supported on os with old nfs-utils, customers may not like seeing the error for every nfs share which is tried for mounting. If 20 shares in config then he will see error 20 times which is not a smooth and pleasant experience. However if both pnfs and regular nfs fail mounting then its serious and admin should know the error, so we use mnt_errors dictionary. I will check on using six.text_type.","commit_id":"747b5496f5b843e552065bf6c51f9d7d08b6b70e"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"4143a69b1a2cd21ce9b90863149b7d6994545019","unresolved":false,"context_lines":[{"line_number":153,"context_line":"            pos \u003d pos + 1"},{"line_number":154,"context_line":"            if re.match(opt_pattern, opt, flags\u003d0):"},{"line_number":155,"context_line":"                return pos"},{"line_number":156,"context_line":"        return 0"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    def _update_option(self, options, option, value\u003dNone):"},{"line_number":159,"context_line":"        \"\"\"Update option if exists else adds it and returns new options.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAUX%2F%2FdxU%3D","line":156,"updated":"2014-02-13 01:00:17.000000000","message":"Why are you returning 0 or pos here when it is called just looking for existence.  Would make more sense to do True and False or something like that.","commit_id":"747b5496f5b843e552065bf6c51f9d7d08b6b70e"},{"author":{"_account_id":6094,"name":"Navneet Singh","email":"navsinghnitd@gmail.com","username":"singn"},"change_message_id":"3b629f0799e0cd944609a18b9f9b3b8628a0f4e9","unresolved":false,"context_lines":[{"line_number":153,"context_line":"            pos \u003d pos + 1"},{"line_number":154,"context_line":"            if re.match(opt_pattern, opt, flags\u003d0):"},{"line_number":155,"context_line":"                return pos"},{"line_number":156,"context_line":"        return 0"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    def _update_option(self, options, option, value\u003dNone):"},{"line_number":159,"context_line":"        \"\"\"Update option if exists else adds it and returns new options.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAUX%2F%2Fces%3D","line":156,"in_reply_to":"AAAAUX%2F%2FdxU%3D","updated":"2014-02-13 06:38:46.000000000","message":"The same function is referred by _update_option which requires position. Returning true/false will not address getting the position back. It also helps to keep code short and simple.","commit_id":"747b5496f5b843e552065bf6c51f9d7d08b6b70e"}]}