Skip to content

Revamp streaming #253

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

Closed
wants to merge 17 commits into from
Closed

Revamp streaming #253

wants to merge 17 commits into from

Conversation

theengineear
Copy link
Contributor

Todo:

  • setup a mock streaming server to test?
  • make a fully async producer/consumer cycle for streaming
  • buffer streamed data so that user knows what data wasn't sent
  • implement our suggested linear/exponential back off rates (with eventual exceptions)
  • allow reconnect-on for both http error codes and socket exceptions

This borrows from chunked requests, but moves the functionality so we don't need that extra submodule. It also attempts to clean up some of the code and make the reconnecting functionality more flexible.

The asyncio module comes standard in 3.4 and can be back ported to 3.3. trollius is a stand-in for asyncio that can be back ported into 2.7.

Though there's still a fair amount of low-level interaction, this attempts to make the interface a bit higher-level.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@theengineear
Copy link
Contributor Author

@chriddyp thoughts on this? I've been playing around with coroutines in some of my hacking-time recently and thought I'd see if I could make our streaming use 'em!

This streaming setup allows users to write to a queue regardless of connection state. You simply use the .write and the background thread writes from the queue to the socket as soon as data is available.

Additionally, it handles heart-beats on it's own instead of allowing our servers to shoot back a 408 code.

@theengineear
Copy link
Contributor Author

We get a fair amount of feedback emails related to streaming, so I thought it might be worth looking into for our 1.7 release.

@chriddyp
Copy link
Member

chriddyp commented Jul 7, 2015

super(ClosedConnection, self).__init__(self.message)


class TooManyConnectFailures(PlotlyConnectionError):
Copy link
Member

Choose a reason for hiding this comment

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

nice

@chriddyp
Copy link
Member

chriddyp commented Jul 7, 2015

i probably won't have time for a while to do a deeper dive review of this, looks cool though! careful around the socket error numbers, that functionality in chunked_requests took me a while to figure out and get right with all the exception cases on both windows and mac. hopefully that test server with circle will be able to grab most of these.

@theengineear
Copy link
Contributor Author

ah, nice test server!

yeah, i saw a lot of commenting around those socket error numbers and figured that some serious time was poured into that :) i'll definitely take a closer peek before trying to get this merged in.

@theengineear
Copy link
Contributor Author

Man, bummed this didn't get in, o well. Maybe i'll have time to revisit down the road.

@theengineear theengineear mentioned this pull request Dec 29, 2016
10 tasks
@nicolaskruchten nicolaskruchten deleted the revamp-streaming branch June 19, 2020 16:09
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.

None yet

2 participants