Merge lp:~robru/friends/streams into lp:friends

[Robert Bruce Park]

Hey barry, it occurred to me that we are just dumping all of our messages into the 'messages' stream, regardless of what type of message it really is, and this makes it kinda difficult for GUI frontends to distinguish between different types of messages.

So what I've done here is enabled a couple of streams, named 'private' and 'search/*'. The private one is for holding private messages, and then any search will create a new 'search/[query]' stream. This thus makes it possible for a frontend to have a new tab for each search, because before, any search results would just be dumped directly into the messages stream, and show up intermingled with normal messages.

In the process of implementing this, I noticed that Twitter is the only protocol for which we've implemented searching, so implementing search on the other protocols should probably be a pretty high priority in the coming week. ;-)

[Ken VanDine (ken-vandine)]

Looks good and yay tests!

review: Approve

[Barry Warsaw (barry)]

I see that Ken merged already, which is great.

I have just a few questions, but it doesn't necessary mean anything needs to
change.

=== modified file 'friends/protocols/twitter.py'
--- friends/protocols/twitter.py 2012-10-12 19:36:46 +0000
+++ friends/protocols/twitter.py 2012-10-12 23:14:17 +0000
> @@ -291,7 +291,7 @@
>
> response = self._get_url('{}?q={}'.format(url, quote(query, safe='')))
> for tweet in response.get(self._search_result_key, []):
> - self._publish_tweet(tweet)
> + self._publish_tweet(tweet, stream='search/{}'.format(query))
>
>
> class RateLimiter(BaseRateLimiter):

This means that the entire search string will be included in the stream name.
Could this cause any problems for database queries? Do we assign any
additional semantics to the contents of the 'stream' column? Or put it
another way, do we treat the stream column as opaque, or do we assign some
semantics to it?

If the contents are opaque, I think that's fine, otherwise we should at least
document the basic semantics of its value. For example, in this case if a
client wanted to know what the search term was, it would have to do a single
left split on slash, e.g.:

    search, slash, query = search_column.partition('/')
    if slash != '/':
        # No separator

This means of course that the query itself could have a slash in it, but only
if we document that just the left-most slash separates the stream name from
the query term.

We're also assuming of course that there are no restrictions on the values of
string columns in Dee.SharedModel. Is that the case?

[Robert Bruce Park (robru)]

On 12-10-15 12:46 PM, Barry Warsaw wrote:
> === modified file 'friends/protocols/twitter.py'
> --- friends/protocols/twitter.py 2012-10-12 19:36:46 +0000
> +++ friends/protocols/twitter.py 2012-10-12 23:14:17 +0000
>> @@ -291,7 +291,7 @@
>>
>> response = self._get_url('{}?q={}'.format(url, quote(query, safe='')))
>> for tweet in response.get(self._search_result_key, []):
>> - self._publish_tweet(tweet)
>> + self._publish_tweet(tweet, stream='search/{}'.format(query))
>>
>>
>> class RateLimiter(BaseRateLimiter):
>
> This means that the entire search string will be included in the stream name.

Yep. The idea was that Gwibber's frontend UI (or any other frontend)
would have different tabs for different streams. Or if not 'tabs' there
would at least be some visual distinction between "normal messages" and
"search results", so the 'streams' column is just a technical way for
the frontend to distinguish the messages.

> Could this cause any problems for database queries? Do we assign any
> additional semantics to the contents of the 'stream' column? Or put it
> another way, do we treat the stream column as opaque, or do we assign some
> semantics to it?

I'm not sure what kind of semantics you mean, but in my mind the stream
column is simply an identifier that helps the frontends distinguish
different types of messages.

> If the contents are opaque, I think that's fine, otherwise we should at least
> document the basic semantics of its value. For example, in this case if a
> client wanted to know what the search term was, it would have to do a single
> left split on slash, e.g.:
>
> search, slash, query = search_column.partition('/')
> if slash != '/':
> # No separator
>
> This means of course that the query itself could have a slash in it, but only
> if we document that just the left-most slash separates the stream name from
> the query term.

Right. I wasn't anticipating that the clients would need to *parse* the
stream column, I was thinking more along the lines that the client would
initiate a search, and then the client would construct it's own
'search/query' string in order to filter the sharedmodel based on that
value, eg, for the purposes of displaying a tab that contains only the
search results, not the entire model.

> We're also assuming of course that there are no restrictions on the values of
> string columns in Dee.SharedModel. Is that the case?

I assume so. Maybe Ken could comment?

[Barry Warsaw (barry)]

On Oct 16, 2012, at 03:13 AM, Robert Bruce Park wrote:

>Right. I wasn't anticipating that the clients would need to *parse* the
>stream column, I was thinking more along the lines that the client would
>initiate a search, and then the client would construct it's own
>'search/query' string in order to filter the sharedmodel based on that
>value, eg, for the purposes of displaying a tab that contains only the
>search results, not the entire model.

That's actually kind of nice. The only trick is that the clients and backend
will have to agree on how those stream names are constructed, which, using the
current concatenation algorithm would be pretty simple. It gets a little more
complicated if there are restrictions on the column values such that they'd
have to be encoded somehow.

I'd actually be kind of surprised if that were the case though.

=== modified file 'friends/protocols/twitter.py'
--- friends/protocols/twitter.py	2012-10-13 01:27:15 +0000
+++ friends/protocols/twitter.py	2012-10-13 01:38:18 +0000
@@ -98,7 +98,7 @@
         return get_json(url, params=data, headers=headers, method=method,
                         rate_limiter=self._rate_limiter)
 
-    def _publish_tweet(self, tweet):
+    def _publish_tweet(self, tweet, stream='messages'):
         """Publish a single tweet into the Dee.SharedModel."""
         tweet_id = tweet.get('id_str')
         if tweet_id is None:
@@ -111,7 +111,7 @@
             message_id=tweet_id,
             message=tweet.get('text', ''),
             timestamp=iso8601utc(parsetime(tweet.get('created_at', ''))),
-            stream='messages',
+            stream=stream,
             sender=user.get('name', ''),
             sender_nick=screen_name,
             from_me=(screen_name == self._account.user_name),
@@ -172,11 +172,11 @@
         """Gather the direct messages sent to/from us."""
         url = self._api_base.format(endpoint='direct_messages')
         for tweet in self._get_url(url):
-            self._publish_tweet(tweet)
+            self._publish_tweet(tweet, stream='private')
 
         url = self._api_base.format(endpoint='direct_messages/sent')
         for tweet in self._get_url(url):
-            self._publish_tweet(tweet)
+            self._publish_tweet(tweet, stream='private')
 
     @feature
     def receive(self):
@@ -205,7 +205,7 @@
         except HTTPError as error:
             log.error('{}: Does that user follow you?'.format(error))
         else:
-            self._publish_tweet(tweet)
+            self._publish_tweet(tweet, stream='private')
 
 # https://dev.twitter.com/docs/api/1.1/post/statuses/update
     @feature
@@ -291,7 +291,7 @@
 
         response = self._get_url('{}?q={}'.format(url, quote(query, safe='')))
         for tweet in response.get(self._search_result_key, []):
-            self._publish_tweet(tweet)
+            self._publish_tweet(tweet, stream='search/{}'.format(query))
 
 
 class RateLimiter(BaseRateLimiter):

=== modified file 'friends/tests/test_identica.py'
--- friends/tests/test_identica.py	2012-10-13 01:27:15 +0000
+++ friends/tests/test_identica.py	2012-10-13 01:38:18 +0000
@@ -117,7 +117,7 @@
 
         self.protocol.private()
 
-        publish.assert_called_with('tweet')
+        publish.assert_called_with('tweet', stream='private')
         self.assertEqual(
             get_url.mock_calls,
             [mock.call('http://identi.ca/api/direct_messages.json'),
@@ -129,7 +129,7 @@
 
         self.protocol.send_private('pumpichank', 'Are you mocking me?')
 
-        publish.assert_called_with('tweet')
+        publish.assert_called_with('tweet', stream='private')
         get_url.assert_called_with(
             'http://identi.ca/api/direct_messages/new.json',
             dict(text='Are you mocking me?', screen_name='pumpichank'))
@@ -221,6 +221,6 @@
 
         self.protocol.search('hello')
 
-        publish.assert_called_with('tweet')
+        publish.assert_called_with('tweet', stream='search/hello')
         get_url.assert_called_with(
             'http://identi.ca/api/search.json?q=hello')

=== modified file 'friends/tests/test_twitter.py'
--- friends/tests/test_twitter.py	2012-10-13 01:27:15 +0000
+++ friends/tests/test_twitter.py	2012-10-13 01:38:18 +0000
@@ -236,7 +236,7 @@
 
         self.protocol.private()
 
-        publish.assert_called_with('tweet')
+        publish.assert_called_with('tweet', stream='private')
         self.assertEqual(
             get_url.mock_calls,
             [mock.call('https://api.twitter.com/1.1/direct_messages.json'),
@@ -249,7 +249,7 @@
 
         self.protocol.send_private('pumpichank', 'Are you mocking me?')
 
-        publish.assert_called_with('tweet')
+        publish.assert_called_with('tweet', stream='private')
         get_url.assert_called_with(
             'https://api.twitter.com/1.1/direct_messages/new.json',
             dict(text='Are you mocking me?', screen_name='pumpichank'))
@@ -356,13 +356,13 @@
 
         self.protocol.tag('yegbike')
 
-        publish.assert_called_with('tweet')
+        publish.assert_called_with('tweet', stream='search/#yegbike')
         get_url.assert_called_with(
             'https://api.twitter.com/1.1/search/tweets.json?q=%23yegbike')
 
         self.protocol.tag('#yegbike')
 
-        publish.assert_called_with('tweet')
+        publish.assert_called_with('tweet', stream='search/#yegbike')
         get_url.assert_called_with(
             'https://api.twitter.com/1.1/search/tweets.json?q=%23yegbike')
 
@@ -373,7 +373,7 @@
 
         self.protocol.search('hello')
 
-        publish.assert_called_with('tweet')
+        publish.assert_called_with('tweet', stream='search/hello')
         get_url.assert_called_with(
             'https://api.twitter.com/1.1/search/tweets.json?q=hello')