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

Streaming: disconnect on keep_alive() #897

Closed
luafran opened this issue Jun 16, 2017 · 4 comments
Closed

Streaming: disconnect on keep_alive() #897

luafran opened this issue Jun 16, 2017 · 4 comments
Labels
Improvement This is regarding an improvement to an existing feature
Milestone

Comments

@luafran
Copy link

luafran commented Jun 16, 2017

Hi

I have a use case where I want to connect to the streaming API for a period of time and then disconnect even when no tweet is received during that period.

When no tweet is received, I can check if defined duration has elapsed in StreamListener keep_alive(), but there is no mechanism to disconnect in this case since _read_loop() is not reading any return value from keep_alive() and there is no way to break the loop in this situation.

As a workaround I am rising a custom exception in my implementation of keep_alive() but this is not ideal, I understand that a better solution is to let keep_alive() return a boolean that when is false breaks the read loop, same as on_data(), on_status(), etc.

@anshudwibhashi
Copy link

I support this. I'd too rather have the keep_alive() function return a boolean to determine whether or not the connection continues to exist. Currently, my workaround is to raise an exception in the keep_alive function, and override the on_exception function to return false.

@aalberti333
Copy link

I would also like to see this implemented.

lbesuchet pushed a commit to lbesuchet/tweepy that referenced this issue Sep 19, 2018
Allow to close stream on keep-alive messages.

This fix allows to disconnect the stream at the next keep-alive message received.
This is possible by either calling disconnect() or having keep_alive() returning False.
@Harmon758
Copy link
Member

This should be resolved with 37e701e by allowing Stream.disconnect to disconnect the stream on the next keep-alive signal received.

I opted not to use the approach of allowing a False return value from the stream listener's on_keep_alive method to indicate that the stream should disconnect, as I intend to remove this for other stream listener methods in Tweepy v4.0 because it's an unclear functionality that should be considered an anti-pattern / bad practice. When necessary, explicitly calling the Stream.disconnect method in the relevant stream listener method is clearer and more straightforward.

I also intend to merge StreamListener into Stream in Tweepy v4.0, which will make doing this simpler and less recursive, i.e. using self.disconnect in the relevant listener method rather than requiring the Stream instance to be available in the relevant stream listener method.

Afterwards, I anticipate being able to simplify the handling of each line of data, so as to allow the stream to disconnect immediately if Stream.disconenct is called in the on_keep_alive method, rather than waiting until the next keep-alive signal (or other line of data).

@Harmon758 Harmon758 added the Improvement This is regarding an improvement to an existing feature label Jan 22, 2021
@Harmon758 Harmon758 added this to the 4.0 milestone Jan 22, 2021
@Harmon758
Copy link
Member

I intend to remove this for other stream listener methods in Tweepy v4.0

This is now done, with 7fc85f6, 7756fa8, and e348c36.

I also intend to merge StreamListener into Stream in Tweepy v4.0

This is now done with 39abff4.

Afterwards, I anticipate being able to simplify the handling of each line of data, so as to allow the stream to disconnect immediately if Stream.disconenct is called in the on_keep_alive method, rather than waiting until the next keep-alive signal (or other line of data).

This was also done with 7756fa8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement This is regarding an improvement to an existing feature
Projects
None yet
Development

No branches or pull requests

4 participants