fix error handling

.coveragerc

@@ -0,0 +1,3 @@
+[run]
+
+include: pydiscourse/*

README.md

@@ -27,6 +27,7 @@ Command line
 
 To help experiment with the Discourse API, pydiscourse provides a simple command line client::
 
-    pydiscourse --host=http://yourhost --api-username=system --api-key=API_KEY latest_topics
-    pydiscourse --host=http://yourhost --api-username=system --api-key=API_KEY topics_by johnsmith
-    pydiscourse --host=http://yourhost --api-username=system --api-key=API_KEY user eviltrout
+    export DISCOURSE_API_KEY=your_master_key
+    pydiscoursecli --host=http://yourhost --api-username=system latest_topics
+    pydiscoursecli --host=http://yourhost --api-username=system topics_by johnsmith
+    pydiscoursecli --host=http://yourhost --api-username=system user eviltrout

pydiscourse/client.py

@@ -1,8 +1,21 @@
 #!/usr/bin/env python
+import logging
+
 import requests
 
 
+log = logging.getLogger('pydiscourse.client')
+
+HTTPError = requests.HTTPError
+
+
+class DiscourseAPIError(Exception):
+    pass
+
+
 class DiscourseClient(object):
+    """ A basic client for the Discourse API that implements the raw API
+    """
     def __init__(self, host, api_username, api_key):
         self.host = host
         self.api_username = api_username
@@ -11,19 +24,25 @@ def __init__(self, host, api_username, api_key):
     def user(self, username):
         return self.get('/users/{0}.json'.format(username))['user']
 
-    def create_user(self, name, username, email, password):
-        r = self.get('users/hp.json')
+    def create_user(self, name, username, email, password='', **kwargs):
+        """ active='true', to avoid sending activation emails
+        """
+        r = self.get('/users/hp.json')
         challenge = r['challenge'][::-1]  # reverse challenge, discourse security check
         confirmations = r['value']
-        return self.post('users', name=name, username=username, email=email,
-                  password=password, password_confirmation=confirmations, challenge=challenge)
+        return self.post('/users', name=name, username=username, email=email,
+                  password=password, password_confirmation=confirmations, challenge=challenge, **kwargs )
 
     def activate_user(self, user_id):
-        return self.put('/admin/users/{0:d}/activate'.format(user_id))
+        return self.put('/admin/users/{0}/activate'.format(user_id))
+
+    def update_avatar(self, username, avatar):
+        """
+        avatar: URL pointing to the image for the users avatar
 
-    def update_avatar(self, username, image):
-        with open(image) as f:
-            return self.put('/users/{0}/preferences/avatar'.format(username), file=f)
+        XXX should be able to pass in the bytes for an image but it doesn't seem to be working properly
+        """
+        return self.post('/users/{0}/preferences/avatar'.format(username), file=avatar)
 
     def update_email(self, username, email):
         return self.put('/users/{0}/preferences/email'.format(username), email=email)
@@ -34,53 +53,79 @@ def update_user(self, username, **kwargs):
     def update_username(self, username, new_username):
         return self.put('/users/{0}/preferences/username'.format(username), username=new_username)
 
-    def hot_topics(self, **params):
-        return self.get('/hot.json', **params)
+    def hot_topics(self, **kwargs):
+        return self.get('/hot.json', **kwargs)
 
-    def latest_topics(self, **params):
-        return self.get('/latest.json', **params)
+    def latest_topics(self, **kwargs):
+        return self.get('/latest.json', **kwargs)
 
-    def new_topics(self, **params):
-        return self.get('/new.json', **params)
+    def new_topics(self, **kwargs):
+        return self.get('/new.json', **kwargs)
 
-    def topic(self, topic_id, **params):
-        return self.get('/t/{0:d}.json'.format(topic_id), **params)
+    def topic(self, topic_id, **kwargs):
+        return self.get('/t/{0}.json'.format(topic_id), **kwargs)
 
-    def topics_by(self, username, **params):
+    def create_post(self, content, **kwargs):
+        return self.post('/posts', raw=content, archtype='regular', **kwargs)
+
+    def topics_by(self, username, **kwargs):
         url = '/topics/created-by/{0}.json'.format(username)
-        return self.get(url, **params)['topic_list']['topics']
+        return self.get(url, **kwargs)['topic_list']['topics']
 
     def invite_user_to_topic(self, user_email, topic_id):
-        params = {
+        kwargs = {
             'email': user_email,
             'topic_id': topic_id,
         }
-        return self.post('/t/{0:d}/invite.json'.format(topic_id), params)
+        return self.post('/t/{0}/invite.json'.format(topic_id), **kwargs)
+
+    def search(self, term, **kwargs):
+        kwargs['term'] = term
+        return self.get('/search.json', **kwargs)
 
-    def search(self, term, **params):
-        params['term'] = term
-        return self.get('/search.json', **params)
+    def categories(self, **kwargs):
+        return self.get('/categories.json', **kwargs)['category_list']['categories']
 
-    def categories(self, **params):
-        return self.get('/categories.json', **params)['category_list']['categories']
+    def site_settings(self, **kwargs):
+        for setting, value in kwargs.items():
+            setting = setting.replace(' ', '_')
+            self._request('PUT', '/admin/site_settings/{0}'.format(setting), {setting: value})
 
-    def get(self, path, **params):
-        return self._request('GET', path, params).json()
+    def get(self, path, **kwargs):
+        return self._request('GET', path, kwargs)
 
-    def put(self, path, **params):
-        # activate_user has no content in response, so no .json() for now
-        return self._request('PUT', path, params)
+    def put(self, path, **kwargs):
+        return self._request('PUT', path, kwargs)
 
-    def post(self, path, **params):
-        return self._request('POST', path, params).json()
+    def post(self, path, **kwargs):
+        return self._request('POST', path, kwargs)
 
-    def delete(self, path, **params):
-        return self._request('DELETE', path, params).json()
+    def delete(self, path, **kwargs):
+        return self._request('DELETE', path, kwargs)
 
     def _request(self, verb, path, params):
         params['api_key'] = self.api_key
         params['api_username'] = self.api_username
         url = self.host + path
-        r = requests.request(verb, url, params=params)
-        r.raise_for_status()
-        return r
+        response = requests.request(verb, url, params=params)
+        log.debug('response %s: %s', response.status_code, repr(response.text))
+        try:
+            response.raise_for_status()
+        except HTTPError:
+            if not response.reason:
+                try:
+                    response.reason = response.json()['errors']
+                except (ValueError, KeyError):
+                    response.reason = response.text
+                response.raise_for_status()
+            raise
+
+        # activate_user has no content in response, so no/u .json() for now
+        if not response.content or response.content == ' ':
+            return None
+
+        decoded = response.json()
+        if 'errors' in decoded:
+            raise DiscourseAPIError(decoded['errors'])
+
+        return decoded

pydiscourse/main.py

@@ -4,8 +4,9 @@
 import optparse
 import pydoc
 import sys
+import os
 
-from pydiscourse.client import DiscourseClient
+from pydiscourse.client import DiscourseClient, HTTPError
 
 
 class DiscourseCmd(cmd.Cmd):
@@ -23,9 +24,15 @@ def __getattr__(self, attr):
 
             def wrapper(arg):
                 args = arg.split()
-                return method(*args)
-
+                kwargs = dict(a.split('=') for a in args if '=' in a)
+                args = [a for a in args if '=' not in a]
+                try:
+                    return method(*args, **kwargs)
+                except HTTPError as e:
+                    print e, e.response.text
+                    return e.response
             return wrapper
+
         elif attr.startswith('help_'):
             method = getattr(self.client, attr[5:])
 
@@ -45,12 +52,12 @@ def postcmd(self, result, line):
 
 def main():
     op = optparse.OptionParser()
-    op.add_option('--host', default='localhost')
+    op.add_option('--host', default='http://localhost:4000')
     op.add_option('--api-user', default='system')
-    op.add_option('--api-key')
 
+    api_key = os.environ['DISCOURSE_API_KEY']
     options, args = op.parse_args()
-    client = DiscourseClient(options.host, options.api_user, options.api_key)
+    client = DiscourseClient(options.host, options.api_user, api_key)
 
     c = DiscourseCmd(client)
     if args:

tests/test_client.py

@@ -35,16 +35,14 @@ def test_user(self, request):
         self.assertRequestCalled(request, 'GET', '/users/someuser.json')
 
     def test_create_user(self, request):
-        pass
+        self.client.create_user('Test User', 'testuser', '[email protected]', 'notapassword')
+        self.assertEqual(request.call_count, 2)
+        # XXX incomplete
 
     def test_activate_user(self, request):
         self.client.activate_user(22)
         self.assertRequestCalled(request, 'PUT', '/admin/users/22/activate')
 
-    def test_activate_user_invalid_id(self, request):
-        with self.assertRaises(ValueError):
-            self.client.activate_user('notavaliduid')
-
     def test_update_email(self, request):
         email = '[email protected]'
         self.client.update_email('someuser', email)
@@ -78,10 +76,6 @@ def test_topic(self, request):
         self.client.topic(22)
         self.assertRequestCalled(request, 'GET', '/t/22.json')
 
-    def test_topic_invalid(self, request):
-        with self.assertRaises(ValueError):
-            self.client.topic('notavalidtopicid')
-
     def test_topics_by(self, request):
         r = self.client.topics_by('someuser')
         self.assertRequestCalled(request, 'GET', '/topics/created-by/someuser.json')
@@ -92,10 +86,6 @@ def invite_user_to_topic(self, request):
         self.client.invite_user_to_topic(email, 22)
         self.assertRequestCalled(request, 'POST', '/t/22/invite.json', email=email, topic_id=22)
 
-    def invite_user_to_topic_invalid_topic(self, request):
-        with self.assertRaises(ValueError):
-            self.client.invite_user_to_topic('someuser', 'invalidtopicid')
-
 
 @mock.patch('requests.request')
 class MiscellaneousTests(ClientBaseTestCase):