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.

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

Improve client socket #112

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

fmcubium
Copy link

Mostly fixes banker.get_client_by_socket() to function as intended. The function should return the correct client regardless of what port they are on as long as they are connected and included in the clients list.

The only problem related to this issue is now that an OSError occurs when comparing the two sockets, saying that an operation was attempted on something that is not a socket. This should be false as running type(socket) returns the expected <class: socket.socket>.

Fixes #77

@adamgulde adamgulde added the question Further information is requested label Feb 18, 2025
@adamgulde
Copy link
Collaborator

I've tried this solution before.. not sure if it works 100%. I said in the TODO message that this wasn't as simple as comparing client.getpeername()[1] with server.getpeername()[1], because I've tried that before. Leaving this PR open until I can sufficiently test it, though. Labeling as question for my own reference later, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make get_client_by_socket() more robust
2 participants