Skip to content

Conversation

@wthrajat
Copy link

@wthrajat wthrajat commented Oct 25, 2024

In src/market/rpc/server.rs, the handle_client() is using raw string parsing for communication. This PR aims to fix it:

  • Use serde for structured data communication instead of raw strings.
  • Added timeouts to the TcpStream using set_read_timeout() and set_write_timeout() to ensure that the server does not hang indefinitely while waiting for data.
  • Introduced one new enum and one new struct:
Request: Handles Post and Get operations from 
Response: Encodes the list of addresses as a JSON response.
  • Added appropriate logging for both Post and Get operations
  • Improve stream handling using BufReader and BufWriter for better I/O operations.
  • JSON encoding/decoding replaces raw string operations for safer and clearer data handling.
  • Fix tests

Signed-off-by: wthrajat <rajatkhanduri290102@gmail.com>
@wthrajat wthrajat self-assigned this Oct 25, 2024
Copy link
Collaborator

@Shourya742 Shourya742 left a comment

Choose a reason for hiding this comment

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

It makes sense to have a Request enum and a Response struct. However, I don’t see any changes in the taker for handling the new serialized format, which could potentially lead to test failures. While using BufWriter makes sense to reduce syscalls, in our case, we're only using the stream for a single write call, so batching writes won’t provide much benefit and will result in similar complexity. That said, the changes look good overall. Just ensure that the taker is handled properly, and double-check if the directory-cli app is functioning correctly.

@wthrajat
Copy link
Author

@Shourya742 Yes it was still supposed to be a draft now. Forgot to change the status.

@wthrajat wthrajat marked this pull request as draft October 25, 2024 15:19
@mojoX911
Copy link

mojoX911 commented Dec 2, 2024

Will be solved later.

@mojoX911 mojoX911 closed this Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants