-
Notifications
You must be signed in to change notification settings - Fork 23
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
Check if connection is running before connecting. #296
base: main
Are you sure you want to change the base?
Conversation
self.periodic_connection_running = True | ||
while not self.stop_connection: | ||
try: | ||
self._connect() | ||
if not self.periodic_connection_running: | ||
self._connect() | ||
self.periodic_connection_running = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure this does what you think it does? This looks like a mutex to me and with this you're defeating the purpose of the mutex?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.periodic_connection_running is just the current state of whether or not we have a current active connection.
self.stop_connection is the mutex-like that actually controls whether or not the connection SHOULD be stopped.
If stop_connection == False and periodic_connection_running == False, we need to restart the connection- anything else we just let pass through, because we're either already connected and should be, or stopping the connection and falling out of the while loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm it doesn't look like this. self.periodic_connection_running
is just prohibiting multiple while
-loops from running. With the change it will be possible to start it multiple times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would multiple while loops be running? That would require calling periodic_connection multiple times (which would raise a runtime error, as you can only start() a thread no more than once).
Even if we did call it more than once, periodic_connection still has the guard against that on line 159.
Finally, a key takeaway here is that we're not running _connect()
from periodic_connection any more than we already were- we're running it less, as we're not running it where we would have before- that is, when we already have a connection, which is the entire cause of the issue people have with 'periodic mode' constantly reconnecting. That's the only real change here.
c6e79b1
to
0c0dcbd
Compare
Any update? |
Don't force-reconnect every delay seconds if the connection is already running.