Fixing streaming, fixes #144 #187

Merged

michaelhelmick merged 3 commits from streaming into master 2013-05-04 12:05:12 -07:00

Conversation 11 Commits 3 Files changed 12 +292 -81

michaelhelmick commented 2013-05-02 12:18:39 -07:00 (Migrated from github.com)

Copy link

Fixes #144

Fixes #144

michaelhelmick commented 2013-05-02 12:24:08 -07:00 (Migrated from github.com)

Copy link

Alright @ryanmcgrath here it is. :D

Something I want your opinion on though:
Right now I have it so they have to use the streaming API like this:

from twython import TwythonStreamer, TwythonStreamHandler

class MyHandler(TwythonStreamHandler):
    def on_error(self, status_code, data):
        print status_code, data

    def on_disconnect(self, data):
        print 'disconnected!'
        print data

stream = TwythonStreamer(app_key, app_secret, oauth_token,
                         oauth_token_secret, MyHandler())

stream.statuses.filter(track='twitter')

Using TwythonStreamer to make calls

Do we want to enable the core Twython class to have streaming access?
So the user can do

from twython import Twython, TwythonStreamHandler

class MyHandler(TwythonStreamHandler):
    def on_error(self, status_code, data):
        print status_code, data

    def on_disconnect(self, data):
        print 'disconnected!'
        print data

stream_config = {
    'handler': MyHandler(),
    'timeout': 500
}
twitter = Twython(app_key, app_secret, oauth_token, oauth_token_secret, stream_config=stream_config)

twitter.stream.statuses.filter(track='twitter')

To me adding it to core Twython might end up getting a bit messing.. having to pass stream_config and all, but let me know if you think you'd want it available from that class!

Alright @ryanmcgrath here it is. :D Something I want your opinion on though: Right now I have it so they have to use the streaming API like this: ``` python from twython import TwythonStreamer, TwythonStreamHandler class MyHandler(TwythonStreamHandler): def on_error(self, status_code, data): print status_code, data def on_disconnect(self, data): print 'disconnected!' print data stream = TwythonStreamer(app_key, app_secret, oauth_token, oauth_token_secret, MyHandler()) stream.statuses.filter(track='twitter') ``` Using TwythonStreamer to make calls Do we want to enable the core `Twython` class to have streaming access? So the user can do ``` python from twython import Twython, TwythonStreamHandler class MyHandler(TwythonStreamHandler): def on_error(self, status_code, data): print status_code, data def on_disconnect(self, data): print 'disconnected!' print data stream_config = { 'handler': MyHandler(), 'timeout': 500 } twitter = Twython(app_key, app_secret, oauth_token, oauth_token_secret, stream_config=stream_config) twitter.stream.statuses.filter(track='twitter') ``` To me adding it to core `Twython` might end up getting a bit messing.. having to pass stream_config and all, but let me know if you think you'd want it available from that class!

michaelhelmick commented 2013-05-02 13:24:21 -07:00 (Migrated from github.com)

Copy link

Also, do you think we should keep all streaming stuff in streaming.py

or make a folder

• streaming
  • `init.py
  • handlers.py
  • api.py
  • types.py

Also, do you think we should keep all streaming stuff in `streaming.py` or make a folder - `streaming` - `**init**.py - `handlers.py` - `api.py` - `types.py`

ryanmcgrath commented 2013-05-02 14:58:49 -07:00 (Migrated from github.com)

Copy link

Hmmm, this is getting really good. The only thing is my original example/pseudo-code was one less deeper in terms of... let's say allocation level. e.g:

class StreamHandler(TwythonStatusFilterStreamer):
    follow = [...]
    delimited = False
    stall_warnings = False
    replies = True

    def onDisconnect(self, code, stream_name, reason):
      # handle disconnect - reconnect?

StreamHandler(...)

vs

class MyHandler(TwythonStreamHandler):
    def on_error(self, status_code, data):
        print status_code, data

    def on_disconnect(self, data):
        print 'disconnected!'
        print data

stream = TwythonStreamer(app_key, app_secret, oauth_token,
                         oauth_token_secret, MyHandler())

My original point was that if we already have a subclass situation going on, we may as well just have them inherit from the handler itself, then instantiate that. Callbacks would basically be abstract methods on the class that a user would opt-in to.

In terms of it crossing streams (lol...) with Twython core, I'd just keep them separate for now - most of the use cases that I see have them fairly separate. Only thing that really exists between the two is the tokens.

As far as the code itself, what I'm looking at seems pretty top notch and I'm overall really impressed with the quality. I think the only thing I'd be concerned about at this point is what the experience is for end-users, ala the above.

Hmmm, this is getting really good. The only thing is my original example/pseudo-code was one less deeper in terms of... let's say allocation level. e.g: ``` python class StreamHandler(TwythonStatusFilterStreamer): follow = [...] delimited = False stall_warnings = False replies = True def onDisconnect(self, code, stream_name, reason): # handle disconnect - reconnect? StreamHandler(...) ``` vs ``` python class MyHandler(TwythonStreamHandler): def on_error(self, status_code, data): print status_code, data def on_disconnect(self, data): print 'disconnected!' print data stream = TwythonStreamer(app_key, app_secret, oauth_token, oauth_token_secret, MyHandler()) ``` My original point was that if we already have a subclass situation going on, we may as well just have them inherit from the handler itself, then instantiate that. Callbacks would basically be abstract methods on the class that a user would opt-in to. In terms of it crossing streams (lol...) with Twython core, I'd just keep them separate for now - most of the use cases that I see have them fairly separate. Only thing that really exists between the two is the tokens. As far as the code itself, what I'm looking at seems pretty top notch and I'm overall really impressed with the quality. I think the only thing I'd be concerned about at this point is what the experience is for end-users, ala the above.

michaelhelmick commented 2013-05-02 15:06:04 -07:00 (Migrated from github.com)

Copy link

Well, not all streams take the same thing, that's why I kept the parameters being passed to each function instead of at class level. (Also, to keep it a bit consistent with Twython core. Twython.updateStatus(status='blah')

Speaking of keeping it consistent. I did diverge though by doing
TwythonStreamer.statuses.filter(track='twitter')
instead of
TwythonStreamer.statuses_filter(track='twitter)

I was going to do it the second way even though all core Twython methods are camelCased
i.e. Twython.updateStatusWithMedia but if you did want me to change it from statuses.filter to statuses_filter I could but just didn't want to do statusesFilter in the case that we move forward with #186

Well, not all streams take the same thing, that's why I kept the parameters being passed to each function instead of at class level. (Also, to keep it a bit consistent with `Twython` core. `Twython.updateStatus(status='blah')` Speaking of keeping it consistent. I did diverge though by doing `TwythonStreamer.statuses.filter(track='twitter')` instead of `TwythonStreamer.statuses_filter(track='twitter)` I was going to do it the second way even though all core `Twython` methods are camelCased i.e. `Twython.updateStatusWithMedia` but if you did want me to change it from `statuses.filter` to `statuses_filter` I could but just didn't want to do `statusesFilter` in the case that we move forward with #186

ryanmcgrath commented 2013-05-02 15:12:30 -07:00 (Migrated from github.com)

Copy link

Mmm, I see your point about the args. My example in my earlier comment has follow, replies, and the like on a Class-level; that's totally fine the way you're doing it. I guess my biggest hangup would be passing the instantiated MyHandler to an instantiation of TwythonStreamer.

I think MyHandler being a subclass of TwythonStreamer would just be cleaner API-wise, and potentially less confusing to someone new to the library. Part of the userbase of this library tends to be people new to programming, after all.

Mmm, I see your point about the args. My example in my earlier comment has `follow`, `replies`, and the like on a Class-level; that's totally fine the way you're doing it. I guess my biggest hangup would be passing the instantiated `MyHandler` to an instantiation of `TwythonStreamer`. I think `MyHandler` being a subclass of `TwythonStreamer` would just be cleaner API-wise, and potentially less confusing to someone new to the library. Part of the userbase of this library tends to be people new to programming, after all.

michaelhelmick commented 2013-05-03 07:25:32 -07:00 (Migrated from github.com)

Copy link

So, you want something like this

streaming.py

class TwythonStreamer(object):
    def __init__(self, app_key, app_secret, oauth_token, oauth_token_secret,
                 handler, timeout=300, retry_count=None, retry_in=10,
                 headers=None):
        """Streaming class for a friendly streaming user experience

        :param app_key: (required) Your applications key
        :param app_secret: (required) Your applications secret key
        :param oauth_token: (required) Used with oauth_token_secret to make
                            authenticated calls
        :param oauth_token_secret: (required) Used with oauth_token to make
                                   authenticated calls
        :param handler: (required) Instance of TwythonStreamHandler to handle
                        stream responses
        :param headers: (optional) Custom headers to send along with the
                        request
        """

        self.auth = OAuth1(app_key, app_secret,
                           oauth_token, oauth_token_secret)

        self.headers = {'User-Agent': 'Twython Streaming v' + __version__}
        if headers:
            self.headers.update(headers)

        self.client = requests.Session()
        self.client.auth = self.auth
        self.client.headers = self.headers
        self.client.stream = True

        self.timeout = timeout

        self.api_version = '1.1'

        self.handler = handler

        self.retry_in = retry_in
        self.retry_count = retry_count

        # Set up type methods
        StreamTypes = TwythonStreamTypes(self)
        self.statuses = StreamTypes.statuses
        self.__dict__['user'] = StreamTypes.user
        self.__dict__['site'] = StreamTypes.site

    def _request(self, url, method='GET', params=None):
        """Internal stream request handling"""
        retry_counter = 0

        method = method.lower()
        func = getattr(self.client, method)

        def _send(retry_counter):
            try:
                if method == 'get':
                    response = func(url, params=params, timeout=self.timeout)
                else:
                    response = func(url, data=params, timeout=self.timeout)
            except requests.exceptions.Timeout:
                self.handler.on_timeout()
            else:
                if response.status_code != 200:
                    self.handler.on_error(response.status_code,
                                          response.content)

                if self.retry_count and (self.retry_count - retry_counter) > 0:
                    time.sleep(self.retry_in)
                    retry_counter += 1
                    _send(retry_counter)

                return response

        response = _send(retry_counter)

        for line in response.iter_lines():
            if line:
                try:
                    self.handler.on_success(json.loads(line))
                except ValueError:
                    raise TwythonStreamError('Response was not valid JSON, \
                                              unable to decode.')

        def on_success(self, data):
            """Called when data has been successfull received from the stream

            Feel free to override this in your own handler.
            See https://dev.twitter.com/docs/streaming-apis/messages for messages
            sent along in stream responses.

            :param data: dict of data recieved from the stream
            """

            if 'delete' in data:
                self.on_delete(data.get('delete'))
            elif 'limit' in data:
                self.on_limit(data.get('limit'))
            elif 'disconnect' in data:
                self.on_disconnect(data.get('disconnect'))

    def on_error(self, status_code, data):
        """Called when stream returns non-200 status code

        :param status_code: Non-200 status code sent from stream
        :param data: Error message sent from stream
        """
        return

    def on_delete(self, data):
        """Called when a deletion notice is received

        Twitter docs for deletion notices: http://spen.se/8qujd

        :param data: dict of data from the 'delete' key recieved from
                     the stream
        """
        return data

    def on_limit(self, data):
        """Called when a limit notice is received

        Twitter docs for limit notices: http://spen.se/hzt0b

        :param data: dict of data from the 'limit' key recieved from
                     the stream
        """
        return data

    def on_disconnect(self, data):
        """Called when a disconnect notice is received

        Twitter docs for disconnect notices: http://spen.se/xb6mm

        :param data: dict of data from the 'disconnect' key recieved from
                     the stream
        """
        return data

    def on_timeout(self):
        return

example code

from twython import TwythonStreamer

class MyStreamer(TwythonStreamer):
    def on_error(self, status_code, data):
        print status_code, data

    def on_disconnect(self, data):
        print 'disconnected!'
        print data

stream = MyStreamer(app_key, app_secret, oauth_token, oauth_token_secret)
stream.statuses.filter(track='twitter')

So, you want something like this `streaming.py` ``` python class TwythonStreamer(object): def __init__(self, app_key, app_secret, oauth_token, oauth_token_secret, handler, timeout=300, retry_count=None, retry_in=10, headers=None): """Streaming class for a friendly streaming user experience :param app_key: (required) Your applications key :param app_secret: (required) Your applications secret key :param oauth_token: (required) Used with oauth_token_secret to make authenticated calls :param oauth_token_secret: (required) Used with oauth_token to make authenticated calls :param handler: (required) Instance of TwythonStreamHandler to handle stream responses :param headers: (optional) Custom headers to send along with the request """ self.auth = OAuth1(app_key, app_secret, oauth_token, oauth_token_secret) self.headers = {'User-Agent': 'Twython Streaming v' + __version__} if headers: self.headers.update(headers) self.client = requests.Session() self.client.auth = self.auth self.client.headers = self.headers self.client.stream = True self.timeout = timeout self.api_version = '1.1' self.handler = handler self.retry_in = retry_in self.retry_count = retry_count # Set up type methods StreamTypes = TwythonStreamTypes(self) self.statuses = StreamTypes.statuses self.__dict__['user'] = StreamTypes.user self.__dict__['site'] = StreamTypes.site def _request(self, url, method='GET', params=None): """Internal stream request handling""" retry_counter = 0 method = method.lower() func = getattr(self.client, method) def _send(retry_counter): try: if method == 'get': response = func(url, params=params, timeout=self.timeout) else: response = func(url, data=params, timeout=self.timeout) except requests.exceptions.Timeout: self.handler.on_timeout() else: if response.status_code != 200: self.handler.on_error(response.status_code, response.content) if self.retry_count and (self.retry_count - retry_counter) > 0: time.sleep(self.retry_in) retry_counter += 1 _send(retry_counter) return response response = _send(retry_counter) for line in response.iter_lines(): if line: try: self.handler.on_success(json.loads(line)) except ValueError: raise TwythonStreamError('Response was not valid JSON, \ unable to decode.') def on_success(self, data): """Called when data has been successfull received from the stream Feel free to override this in your own handler. See https://dev.twitter.com/docs/streaming-apis/messages for messages sent along in stream responses. :param data: dict of data recieved from the stream """ if 'delete' in data: self.on_delete(data.get('delete')) elif 'limit' in data: self.on_limit(data.get('limit')) elif 'disconnect' in data: self.on_disconnect(data.get('disconnect')) def on_error(self, status_code, data): """Called when stream returns non-200 status code :param status_code: Non-200 status code sent from stream :param data: Error message sent from stream """ return def on_delete(self, data): """Called when a deletion notice is received Twitter docs for deletion notices: http://spen.se/8qujd :param data: dict of data from the 'delete' key recieved from the stream """ return data def on_limit(self, data): """Called when a limit notice is received Twitter docs for limit notices: http://spen.se/hzt0b :param data: dict of data from the 'limit' key recieved from the stream """ return data def on_disconnect(self, data): """Called when a disconnect notice is received Twitter docs for disconnect notices: http://spen.se/xb6mm :param data: dict of data from the 'disconnect' key recieved from the stream """ return data def on_timeout(self): return ``` example code ``` python from twython import TwythonStreamer class MyStreamer(TwythonStreamer): def on_error(self, status_code, data): print status_code, data def on_disconnect(self, data): print 'disconnected!' print data stream = MyStreamer(app_key, app_secret, oauth_token, oauth_token_secret) stream.statuses.filter(track='twitter') ```

ryanmcgrath commented 2013-05-03 12:31:43 -07:00 (Migrated from github.com)

Copy link

That is... exactly what I was getting at, wow. Nice job.

What do you think, after seeing it? The aim is to have it be as straight-forward and clear-cut as possible, which I think this does - it's essentially async code, which... well, I don't really think Python does a great job of enabling, but this approach is grok-able at a glance which I think does wonders.

That is... exactly what I was getting at, wow. Nice job. What do you think, after seeing it? The aim is to have it be as straight-forward and clear-cut as possible, which I think this does - it's essentially async code, which... well, I don't really think Python does a great job of enabling, but this approach is grok-able at a glance which I think does wonders.

michaelhelmick commented 2013-05-03 13:21:50 -07:00 (Migrated from github.com)

Copy link

Yeah, that's fine. The only gripe I have is that I think the Handling is separate from the actual Streaming.. but it is kind of weird passing the Handler instance to the streamer -- so I'm fine integrating the handling methods into the Streamer. I'll prob. fix this tomorrow!

Yeah, that's fine. The only gripe I have is that I think the Handling is separate from the actual Streaming.. **but** it is kind of weird passing the Handler instance to the streamer -- so I'm fine integrating the handling methods into the Streamer. I'll prob. fix this tomorrow!

michaelhelmick commented 2013-05-03 14:00:15 -07:00 (Migrated from github.com)

Copy link

Orrrrr... I just got it done. haha. Everything looks kosher to me.

Last thing before I merge and push something tomorrow..

What do you think about the file structure for streaming?
Should I do like I said and do:

• streaming
  • __init__.py
  • api.py
  • types.py

Or just keep everything in streaming.py?

Orrrrr... I just got it done. haha. Everything looks kosher to me. Last thing before I merge and push something tomorrow.. What do you think about the file structure for streaming? Should I do like I said and do: - `streaming` - `__init__.py` - `api.py` - `types.py` Or just keep everything in `streaming.py`?

ryanmcgrath commented 2013-05-03 14:26:20 -07:00 (Migrated from github.com)

Copy link

I would personally separate it, because it's easier to grok for new people, but that's also something we can refine if we need to down the road. Take your pick, I'm fine with either.

Nice job!

I would personally separate it, because it's easier to grok for new people, but that's also something we can refine if we need to down the road. Take your pick, I'm fine with either. Nice job!

michaelhelmick commented 2013-05-03 14:33:53 -07:00 (Migrated from github.com)

Copy link

I moved it into it's own folder! I'll merge and push to PyPi tomorrow.

I moved it into it's own folder! I'll merge and push to PyPi tomorrow.

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

Bug

  

Enhancement

  

Feature Suggestion

No labels

Bug

Enhancement

Feature Suggestion

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: code/twython#187

No description provided.