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

Create multi-threading.py (example) #401

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

joakimed
Copy link

New example for simultaneous sending and receiving, as discussed in the last few comments on #396 .
I'm sure there are better ways of doing this, so please hit me with any ideas for improvement.

Also, the script is a little buggy when it comes to printing. For instance; if you are in the middle of writing something and you receive another message, it gets printed in the middle of what you are writing. I was unable to find a solution without importing a third-party library.

Copy link
Member

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

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

I think we still need to do a bit of research. But again, I'm not really sure if we can recommend a broken solution?
And I don't think you can prevent the input text from being overwritten, without taking control over the terminal. Unless you do some kind of queuing? 😕

But thanks for the PR anyway, it's a nice place to start the discussion! 👍

examples/multi-threading.py Outdated Show resolved Hide resolved
examples/multi-threading.py Outdated Show resolved Hide resolved
examples/multi-threading.py Outdated Show resolved Hide resolved
t1 = threading.Thread(target=receive)
t2 = threading.Thread(target=send)
t1.start()
t2.start()
Copy link
Member

Choose a reason for hiding this comment

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

Do we need a join somewhere? Or is that automatically done?

Copy link
Author

Choose a reason for hiding this comment

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

I removed one thread, and made the remaining thread daemonic. From what I gather this should get destroyed when the main program exits.

examples/multi-threading.py Outdated Show resolved Hide resolved
examples/multi-threading.py Outdated Show resolved Hide resolved
@madsmtm
Copy link
Member

madsmtm commented Feb 28, 2019

I've been investigating a bit, I'd guess that https://github.com/kennethreitz/requests/issues/2766 would make it possible to solve the thread safety problems, but our code needs to be implemented with thread safety in mind as well, so it might not be that simple!

t2.start()
# Clean-up on exit
except KeyboardInterrupt:
client1.logout()
Copy link
Member

Choose a reason for hiding this comment

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

I'd argue there's really not a big need to log out (it's not shown in the other examples, either)

except KeyboardInterrupt:
client1.logout()
client2.logout()
sys.exit(0)
Copy link
Member

Choose a reason for hiding this comment

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

We should clean the client2 up properly instead. That can be done by setting Client.listening to False, and then wait for the thread to clean up afterwards, using Thread.join

Copy link
Member

Choose a reason for hiding this comment

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

And then there'd be no need to make t1 daemonic

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.

2 participants