Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changing logic to return payload as json in the case of any exception depending on received bytes #20

Merged
merged 3 commits into from
Feb 28, 2023
Merged

Changing logic to return payload as json in the case of any exception depending on received bytes #20

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a064ef
Changing logic to return payload as json in the case of any exception…
Feb 10, 2023
b03adcb
fixing pycodestyle issues
Feb 10, 2023
0f119d5
Consolidating exceptions for recv_json methods in both ClientSocket a…
Feb 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 12 additions & 30 deletions dss_datamover/socket_communication.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ def recv_json(self, format="JSON"):
The message contains 10 bytes header and body. Read header untill received 10 bytes or timeout.
Iterate to receive desired number of bytes from socket.
:format: JSON/String
:timeout: default 60 seconds
:return: Return received data in json format.
"""
self.socket.settimeout(self.config.get("socket_options", {}).get("recv_timeout", DEFAULT_RECV_TIMEOUT))
Expand Down Expand Up @@ -200,21 +199,13 @@ def recv_json(self, format="JSON"):
return msg
except socket.timeout as e:
self.logger.error("ClientSocket: Timeout ({} seconds) from recv function".format((datetime.now() - time_started).seconds))
# return status depending on received message size, if incomplete a Runtime Error should have been raised
if len(msg_len_in_bytes) == response_header_length and received_data_size == msg_len:
return json.loads(msg)
else:
return json.loads("{}")
except socket.error as e:
self.logger.error(f"ClientSocket SocketError: {e}")
raise socket.error(f"ClientSocket SocketError: {e}")
Copy link
Contributor

@grandsuri grandsuri Feb 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we are removing all the raise exceptions, then why don't we consolidate all these exceptions into a single one and just log the error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could potentially do that, only downside is that we would lose how long a socket timeout took. If you think it is worth it to simplify the code, then I can go ahead and do that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except socket.timeout, you can bundle rest of the exceptions to a generic exception unless it has some value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good, thank you

except ValueError as e:
raise ValueError(f"ClientSocket: ValueError - {e}")
except RuntimeError as e:
self.logger.error(f"ClientSocket: RuntimeError {e}")
raise RuntimeError(f"ClientSocket: RuntimeError {e}")
except Exception as e:
raise Exception(f"ClientSocket: Exception {e}")
self.logger.error(f"ClientSocket: Exception {e}")

# return status depending on received message size, if incomplete a Runtime Error should have been raised
if len(msg_len_in_bytes) == response_header_length and received_data_size == msg_len:
return json.loads(msg)
return json.loads("{}")

def close(self):
"""
Expand Down Expand Up @@ -323,7 +314,6 @@ def recv_json(self, format="JSON"):
The message contains 10 bytes header and body. Read header untill received 10 bytes or timeout.
Iterate to receive desired number of bytes from socket.
:format: JSON/String
:timeout: default 60 seconds
:return: Return received data in json format.
"""
self.client_socket.settimeout(self.config.get("socket_options", {}).get("recv_timeout", DEFAULT_RECV_TIMEOUT))
Expand Down Expand Up @@ -377,21 +367,13 @@ def recv_json(self, format="JSON"):

except socket.timeout as e:
self.logger.error("ServerSocket: Timeout ({} seconds) from recv function".format((datetime.now() - time_started).seconds))
# return status depending on received message size, if incomplete a Runtime Error should have been raised
if len(msg_len_in_bytes) == response_header_length and received_data_size == msg_len:
return json.loads(msg)
else:
return json.loads("{}")
except socket.error as e:
self.logger.error(f"ServerSocket SocketError: {e}")
raise socket.error(f"ServerSocket SocketError: {e}")
except ValueError as e:
raise ValueError(f"ServerSocket: ValueError - {e}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the top comment applies here as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

responded to comment above

except RuntimeError as e:
self.logger.error(f"ServerSocket: RuntimeError {e}")
raise RuntimeError(f"ServerSocket: RuntimeError {e}")
except Exception as e:
raise Exception(f"ServerSocket: Exception {e}")
self.logger.error(f"ServerSocket: Exception {e}")

# return status depending on received message size, if incomplete a Runtime Error should have been raised
if len(msg_len_in_bytes) == response_header_length and received_data_size == msg_len:
return json.loads(msg)
return json.loads("{}")

def close(self):
"""
Expand Down