)]}'
{"Engine/swift/storlet_gateway/gateways/docker/runtime.py":[{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"9436c2fd0bf79abd597f03fc03e7b783115821d2","unresolved":false,"context_lines":[{"line_number":765,"context_line":"        try:"},{"line_number":766,"context_line":"            self._prepare_invocation_descriptors()"},{"line_number":767,"context_line":"            with self.storlet_logger.activate():"},{"line_number":768,"context_line":"                self._invoke()"},{"line_number":769,"context_line":""},{"line_number":770,"context_line":"            if not self.srequest.has_fd:"},{"line_number":771,"context_line":"                self._wait_for_write_with_timeout(self._input_data_write_fd)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa6399be_1ebb6de6","line":768,"updated":"2016-10-20 08:22:46.000000000","message":"What happens if _invoke raises an exception before calling _close_remote_side_descriptors? Seems like the Exception block below does not call _close_remote_side_descriptors.\nIt seems that the previous version of the code had the same problem as there was no \"with\" statement for _activate_invocation_descriptors, but I might be missing something here.\n\nIf I did get it right, perhaps it is worthwhile to add a unit test making sure _close_remote_side_descriptors is called in the case of an exception raised from _invoke before it gets to call _close_remote_side_descriptors","commit_id":"c41410f1d668fead5a3e1fd25e3d5622bd500aa4"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"174e8f6602dc721ba7e015e65e37095274fae7f4","unresolved":false,"context_lines":[{"line_number":690,"context_line":"            # TODO(takashi): Check the reponse here"},{"line_number":691,"context_line":"            os.read(read_fd, 10)"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"    def _invoke(self):"},{"line_number":694,"context_line":"        \"\"\""},{"line_number":695,"context_line":"        Send an execution command to the remote daemon factory"},{"line_number":696,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_d9c77a10","line":693,"range":{"start_line":693,"start_character":8,"end_line":693,"end_character":15},"updated":"2016-10-31 05:04:00.000000000","message":"IMO, send_execute method should be done inside invoke function.\nPlease consider to rename this func like get_task_id or to include send_execute inside invoke.","commit_id":"92eb66143bebff4601d67e0f31a58834312bc8f5"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"174e8f6602dc721ba7e015e65e37095274fae7f4","unresolved":false,"context_lines":[{"line_number":692,"context_line":""},{"line_number":693,"context_line":"    def _invoke(self):"},{"line_number":694,"context_line":"        \"\"\""},{"line_number":695,"context_line":"        Send an execution command to the remote daemon factory"},{"line_number":696,"context_line":"        \"\"\""},{"line_number":697,"context_line":"        self._wait_for_read_with_timeout(self.execution_str_read_fd)"},{"line_number":698,"context_line":"        # TODO(kota_): need an assertion for task_id format"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_79690e25","line":695,"range":{"start_line":695,"start_character":8,"end_line":695,"end_character":62},"updated":"2016-10-31 05:04:00.000000000","message":"This does not describe what you are doing in this function.","commit_id":"92eb66143bebff4601d67e0f31a58834312bc8f5"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"174e8f6602dc721ba7e015e65e37095274fae7f4","unresolved":false,"context_lines":[{"line_number":699,"context_line":"        self.task_id \u003d os.read(self.execution_str_read_fd, 10)"},{"line_number":700,"context_line":"        os.close(self.execution_str_read_fd)"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"    def _send_execute(self):"},{"line_number":703,"context_line":"        \"\"\""},{"line_number":704,"context_line":"        Send fds to the remote daemon factory"},{"line_number":705,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_f9ca7e19","line":702,"range":{"start_line":702,"start_character":8,"end_line":702,"end_character":21},"updated":"2016-10-31 05:04:00.000000000","message":"how about _send_execute_cmd","commit_id":"92eb66143bebff4601d67e0f31a58834312bc8f5"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"174e8f6602dc721ba7e015e65e37095274fae7f4","unresolved":false,"context_lines":[{"line_number":701,"context_line":""},{"line_number":702,"context_line":"    def _send_execute(self):"},{"line_number":703,"context_line":"        \"\"\""},{"line_number":704,"context_line":"        Send fds to the remote daemon factory"},{"line_number":705,"context_line":"        \"\"\""},{"line_number":706,"context_line":"        dtg \u003d SBusDatagram("},{"line_number":707,"context_line":"            SBUS_CMD_EXECUTE,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_19d2d2d2","line":704,"range":{"start_line":704,"start_character":8,"end_line":704,"end_character":45},"updated":"2016-10-31 05:04:00.000000000","message":"Send execute command to the remote daemon factory to invoke storlet execution","commit_id":"92eb66143bebff4601d67e0f31a58834312bc8f5"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"f01b528f184a2a853215b88345b1cea7941b8dc2","unresolved":false,"context_lines":[{"line_number":695,"context_line":"        Send an execution command to the remote daemon factory"},{"line_number":696,"context_line":"        \"\"\""},{"line_number":697,"context_line":"        with self.storlet_logger.activate(),\\"},{"line_number":698,"context_line":"                self._activate_invocation_descriptors():"},{"line_number":699,"context_line":"            self._send_execute_by_sbus()"},{"line_number":700,"context_line":"        self._wait_for_read_with_timeout(self.execution_str_read_fd)"},{"line_number":701,"context_line":"        # TODO(kota_): need an assertion for task_id format"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba5da102_28a72550","line":698,"range":{"start_line":698,"start_character":16,"end_line":698,"end_character":55},"updated":"2016-11-01 04:55:13.000000000","message":"In this change, we removed duplicate close calls, so I think you can get rid of safe_close in _activate_invocation_descriptors, which let us notice some errors in close operation.","commit_id":"bb7dfc0a0e63cd27e8afd0f20e060ee10ef5127d"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"f01b528f184a2a853215b88345b1cea7941b8dc2","unresolved":false,"context_lines":[{"line_number":702,"context_line":"        self.task_id \u003d os.read(self.execution_str_read_fd, 10)"},{"line_number":703,"context_line":"        os.close(self.execution_str_read_fd)"},{"line_number":704,"context_line":""},{"line_number":705,"context_line":"    def _send_execute_by_sbus(self):"},{"line_number":706,"context_line":"        \"\"\""},{"line_number":707,"context_line":"        Send execute command to the remote daemon factory to invoke storlet"},{"line_number":708,"context_line":"        execution"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba5da102_c8b14991","line":705,"range":{"start_line":705,"start_character":21,"end_line":705,"end_character":29},"updated":"2016-11-01 04:55:13.000000000","message":"I don\u0027t think we need to say \u0027by_sbus\u0027 explicitly.\nCan we name this func just something like \u0027_send_execute_command\u0027?","commit_id":"bb7dfc0a0e63cd27e8afd0f20e060ee10ef5127d"}],"tests/unit/swift/storlet_gateway/gateways/docker/test_runtime.py":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"d91ee630556b277e629a450a7e04e3d0e4667646","unresolved":false,"context_lines":[{"line_number":55,"context_line":"            self.rbuf \u003d self.rbuf[size:]"},{"line_number":56,"context_line":"            return ret"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"        def close(self):"},{"line_number":59,"context_line":"            self.closed \u003d True"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def fake_os_read(fd, size):"},{"line_number":62,"context_line":"        return fd.read(size)"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba5da102_155275dd","line":59,"range":{"start_line":58,"start_character":0,"end_line":59,"end_character":30},"updated":"2016-11-01 06:57:02.000000000","message":"This close does not simulate OSError for duplicate close. IMO you should change this to confirm your change.","commit_id":"5272ec777b82df750f70db59af39fad474e553e1"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"f93b93e0441ff608a5182e8fb4887f2340660aa8","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"        def close(self):"},{"line_number":59,"context_line":"            if self.closed:"},{"line_number":60,"context_line":"                raise OSError(\u00279\u0027, \u0027Bad file descriptor\u0027)"},{"line_number":61,"context_line":"            self.closed \u003d True"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def fake_os_read(fd, size):"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5da102_d5a8d9dd","line":60,"range":{"start_line":60,"start_character":30,"end_line":60,"end_character":33},"updated":"2016-11-02 02:42:58.000000000","message":"Please use existing definition in errno. Also, can you confirm the order for these two parameters?","commit_id":"a3a7f0801b669696fff1b6544918f13623f32004"}]}