Merge lp:~matiasb/click-toolbelt/added-missing-params into lp:click-toolbelt

[Matias Bordese]

- Added missing parameters (architectures, changelog).
- Updated/fixed SSO login integration to work with updated ssoclient.

Added missing parameters (architectures, what's new).
Updated/fixed SSO login integration to work with updated ssoclient.

[Ricardo Kirkner (ricardokirkner)]

LGTM

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~matiasb/click-toolbelt/added-missing-params into lp:click-toolbelt failed. Below is the output from the failed tests.

running test
Checking .pth file support in .
/usr/bin/python -E -c pass
Searching for ssoclient
Reading http://pypi.python.org/simple/ssoclient/
Best match: ssoclient 2.0
Downloading https://pypi.python.org/packages/source/s/ssoclient/ssoclient-2.0.tar.gz#md5=a4c7a3aeb875d5d245b58752aa680494
Processing ssoclient-2.0.tar.gz
Writing /tmp/easy_install-Ky0kSO/ssoclient-2.0/setup.cfg
Running ssoclient-2.0/setup.py -q bdist_egg --dist-dir /tmp/easy_install-Ky0kSO/ssoclient-2.0/egg-dist-tmp-lw9Ja6

Installed /mnt/tarmac/cache/click-toolbelt/trunk/ssoclient-2.0-py2.7.egg
Searching for requests-oauthlib
Reading http://pypi.python.org/simple/requests-oauthlib/
Best match: requests-oauthlib 0.4.0
Downloading https://pypi.python.org/packages/source/r/requests-oauthlib/requests-oauthlib-0.4.0.tar.gz#md5=2649560205a02473998e0e7dd881ee5a
Processing requests-oauthlib-0.4.0.tar.gz
Writing /tmp/easy_install-X3uWbE/requests-oauthlib-0.4.0/setup.cfg
Running requests-oauthlib-0.4.0/setup.py -q bdist_egg --dist-dir /tmp/easy_install-X3uWbE/requests-oauthlib-0.4.0/egg-dist-tmp-mfVvSD

Installed /mnt/tarmac/cache/click-toolbelt/trunk/requests_oauthlib-0.4.0-py2.7.egg
Searching for cliff
Reading http://pypi.python.org/simple/cliff/
Best match: cliff 1.4.5
Downloading https://pypi.python.org/packages/source/c/cliff/cliff-1.4.5.tar.gz#md5=21743e9e800cf526e88e420f366a4c5c
Processing cliff-1.4.5.tar.gz
Writing /tmp/easy_install-GPmDRK/cliff-1.4.5/setup.cfg
Running cliff-1.4.5/setup.py -q bdist_egg --dist-dir /tmp/easy_install-GPmDRK/cliff-1.4.5/egg-dist-tmp-_Iaxba

Installed /mnt/tarmac/cache/click-toolbelt/trunk/cliff-1.4.5-py2.7.egg

zip_safe flag not set; analyzing archive contents...
warning: no files found matching '*.py' under directory 'tests'
Traceback (most recent call last):
  File "setup.py", line 69, in <module>
    tests_require=['mock'],
  File "/usr/lib/python2.7/distutils/core.py", line 152, in setup
    dist.run_commands()
  File "/usr/lib/python2.7/distutils/dist.py", line 953, in run_commands
    self.run_command(cmd)
  File "/usr/lib/python2.7/distutils/dist.py", line 972, in run_command
    cmd_obj.run()
  File "/usr/lib/python2.7/dist-packages/setuptools/command/test.py", line 128, in run
    self.distribution.fetch_build_eggs(self.distribution.install_requires)
  File "/usr/lib/python2.7/dist-packages/setuptools/dist.py", line 245, in fetch_build_eggs
    parse_requirements(requires), installer=self.fetch_build_egg
  File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 598, in resolve
    raise VersionConflict(dist,req) # XXX put more info here
pkg_resources.VersionConflict: (requests 1.2.3 (/usr/lib/python2.7/dist-packages), Requirement.parse('requests>=2.0.0'))

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~matiasb/click-toolbelt/added-missing-params into lp:click-toolbelt failed. Below is the output from the failed tests.

rm -rf .env
rm -rf cliff-*.egg
rm -rf cmd2-*.egg
rm -rf mock-*.egg
rm -rf oauthlib-*.egg
rm -rf prettytable-*.egg
rm -rf pyparsing-*.egg
rm -rf pyxdg-*.egg
rm -rf requests-*.egg
rm -rf requests_oauthlib-*.egg
rm -rf ssoclient-*.egg
rm -rf click.egg-info
virtualenv .env

make: virtualenv: Command not found
make: *** [env] Error 127

=== modified file 'README'
--- README	2013-09-18 08:15:02 +0000
+++ README	2013-10-09 14:25:04 +0000
@@ -34,7 +34,7 @@
 
 ::
 
-    $ click upload <namespace>.<app_name> <version> <filename>
+    $ click upload <namespace>.<app_name> <version> <filename> <changelog> [architecture]
 
 
 For this command to be successful, certain criteria must be met::
@@ -42,3 +42,5 @@
     - user is registered as a developer on myapps.developer.ubuntu.com
     - the application has been previously created on myapps.developer.ubuntu.com
     - user owns the application for which he's upload the click package
+    - if not specified, architecture will default to 'all'
+      (possible values: 'armhf', 'i386', 'amd64', 'all'; you can specify more than one)

=== modified file 'click/login.py'
--- click/login.py	2013-09-18 12:00:15 +0000
+++ click/login.py	2013-10-09 14:25:04 +0000
@@ -7,6 +7,7 @@
 from cliff.command import Command
 from ssoclient.v2 import (
     ApiException,
+    UnexpectedApiError,
     V2ApiClient,
 )
 from xdg.BaseDirectory import save_config_path
@@ -59,10 +60,12 @@
         client = V2ApiClient(endpoint=api_endpoint)
         try:
             response = client.login(data=data)
+            result['body'] = response
             result['success'] = True
-            result['body'] = response.json()
         except ApiException as err:
             result['body'] = err.body
+        except UnexpectedApiError as err:
+            result['body'] = err.json_body
         return result
 
     def take_action(self, parsed_args):

=== modified file 'click/tests/test_login.py'
--- click/tests/test_login.py	2013-09-18 12:00:15 +0000
+++ click/tests/test_login.py	2013-10-09 14:25:04 +0000
@@ -112,8 +112,8 @@
         self.assertTrue(cfg.has_section('login.ubuntu.com'))
         self.assertEqual(dict(cfg.items('login.ubuntu.com')), data)
 
-    @patch('click.login.V2ApiClient.login')
-    def test_take_action_successful(self, mock_login):
+    @patch('ssoclient.v2.http.requests.Session.request')
+    def test_take_action_successful(self, mock_post):
         token_data = {
             'consumer_key': 'consumer-key',
             'consumer_secret': 'consumer-secret',
@@ -123,32 +123,56 @@
         response = Response()
         response.status_code = 201
         response._content = json.dumps(token_data).encode('utf-8')
-        mock_login.return_value = response
+        mock_post.return_value = response
 
         data = {
             'email': '[e-mail]',
             'password': 'password',
+            'token_name': 'consumer-key',
         }
         result = self.command.login(data)
         expected = {'success': True, 'body': token_data}
         self.assertEqual(result, expected)
 
-    @patch('click.login.V2ApiClient.login')
-    def test_take_action_unsuccessful(self, mock_login):
-        error_data = {
-            'message': 'Error during login.',
-            'code': 'UNAUTHORISED',
-            'extra': {},
-        }
-        response = Response()
-        response.status_code = 401
-        response.reason = 'UNAUTHORISED'
-        response._content = json.dumps(error_data).encode('utf-8')
-        mock_login.side_effect = ApiException(response, response.json())
-
-        data = {
-            'email': '[e-mail]',
-            'password': 'password',
+    @patch('ssoclient.v2.http.requests.Session.request')
+    def test_take_action_unsuccessful_api_exception(self, mock_post):
+        error_data = {
+            'message': 'Error during login.',
+            'code': 'INVALID_CREDENTIALS',
+            'extra': {},
+        }
+        response = Response()
+        response.status_code = 401
+        response.reason = 'UNAUTHORISED'
+        response._content = json.dumps(error_data).encode('utf-8')
+        mock_post.return_value = response
+
+        data = {
+            'email': '[e-mail]',
+            'password': 'password',
+            'token_name': 'consumer-key',
+        }
+        result = self.command.login(data)
+        expected = {'success': False, 'body': error_data}
+        self.assertEqual(result, expected)
+
+    @patch('ssoclient.v2.http.requests.Session.request')
+    def test_take_action_unsuccessful_unexpected_error(self, mock_post):
+        error_data = {
+            'message': 'Error during login.',
+            'code': 'UNEXPECTED_ERROR_CODE',
+            'extra': {},
+        }
+        response = Response()
+        response.status_code = 401
+        response.reason = 'UNAUTHORISED'
+        response._content = json.dumps(error_data).encode('utf-8')
+        mock_post.return_value = response
+
+        data = {
+            'email': '[e-mail]',
+            'password': 'password',
+            'token_name': 'consumer-key',
         }
         result = self.command.login(data)
         expected = {'success': False, 'body': error_data}

=== modified file 'click/tests/test_upload.py'
--- click/tests/test_upload.py	2013-09-13 13:32:25 +0000
+++ click/tests/test_upload.py	2013-10-09 14:25:04 +0000
@@ -27,7 +27,9 @@
         args = None
         self.command = Upload(app, args)
         self.app_name = 'namespace.package_name'
+        self.app_archs = ['all']
         self.app_version = '1'
+        self.app_changelog = 'Initial version.'
         self.uploaded_callback_url = MYAPPS_API_ROOT_URL + 'click-uploaded/1/'
 
         # setup patches
@@ -144,7 +146,9 @@
         self.mock_post.return_value = response
 
         data = self.command.get_upload_data(self.app_name,
-                                            self.app_version)
+                                            self.app_version,
+                                            self.app_changelog,
+                                            self.app_archs)
         self.assertEqual(data, expected)
 
     def test_get_upload_data_for_unknown_app(self):
@@ -155,7 +159,9 @@
         self.mock_post.return_value = response
 
         data = self.command.get_upload_data(self.app_name,
-                                            self.app_version)
+                                            self.app_version,
+                                            self.app_changelog,
+                                            self.app_archs)
         self.assertEqual(data, {
             'success': False,
             'message': 'The requested Click Package could not be found.'})
@@ -164,23 +170,33 @@
         self.mock_post.side_effect = Exception('some http error')
 
         data = self.command.get_upload_data(self.app_name,
-                                            self.app_version)
+                                            self.app_version,
+                                            self.app_changelog,
+                                            self.app_archs)
         self.assertEqual(data, {'success': False,
                                 'message': 'some http error'})
 
     def test_get_upload_data_uses_default_myapps_url(self):
         upload_url = MYAPPS_API_ROOT_URL + "click-upload/%s/" % (
             self.app_name,)
-        self.command.get_upload_data(self.app_name, self.app_version)
+        self.command.get_upload_data(
+            self.app_name, self.app_version, self.app_changelog,
+            self.app_archs)
         self.mock_post.assert_called_once_with(
-            upload_url, data={'version': self.app_version})
+            upload_url, data={'version': self.app_version,
+                              'architectures': self.app_archs,
+                              'changelog': self.app_changelog})
 
     def test_get_upload_data_uses_environment_variable(self):
         upload_url = MYAPPS_API_ROOT_URL + "click-upload/%s/" % (
             self.app_name,)
-        self.command.get_upload_data(self.app_name, self.app_version)
+        self.command.get_upload_data(
+            self.app_name, self.app_version, self.app_changelog,
+            self.app_archs)
         self.mock_post.assert_called_once_with(
-            upload_url, data={'version': self.app_version})
+            upload_url, data={'version': self.app_version,
+                              'architectures': self.app_archs,
+                              'changelog': self.app_changelog})
 
 
 class MarkUploadedTestCase(BaseUploadCommandTestCase):
@@ -286,21 +302,28 @@
         patcher.start()
         self.addCleanup(patcher.stop)
 
+        parsed_args = namedtuple(
+            'parsed_args',
+            'application_name, version, filename, changelog, arch')
+
+        filename = 'package.click'
+        self.args = parsed_args(
+            self.app_name, self.app_version, filename, self.app_changelog,
+            self.app_archs)
+
     def test_parser(self):
         parser = self.command.get_parser(__namespace__)
-        for i, name in enumerate(['application_name', 'version', 'filename']):
+        cmd_args = ['application_name', 'version', 'filename', 'changelog']
+        for i, name in enumerate(cmd_args):
             # argument 0 is builtin --help
             # start comparing from first extra argument
             self.assertEqual(parser._actions[i + 1].dest, name)
-            self.assertFalse(parser._actions[i + 1].required)
+            self.assertTrue(parser._actions[i + 1].required)
+        # arch is optional, and last param
+        self.assertEqual(parser._actions[-1].dest, 'arch')
+        self.assertFalse(parser._actions[-1].required)
 
     def test_take_action(self):
-        parsed_args = namedtuple('parsed_args',
-                                 'application_name, version, filename')
-
-        filename = 'package.click'
-        args = parsed_args(self.app_name, self.app_version, filename)
-
         response = Response()
         response.status_code = 200
         response._content = json.dumps({'successful': True}).encode(
@@ -309,12 +332,13 @@
 
         with patch('click.upload.Upload.mark_uploaded') as mock_mark_uploaded:
             mock_mark_uploaded.return_value = {'success': True}
-            self.command.take_action(args)
+            self.command.take_action(self.args)
 
         # assert request was made to get upload data
         # followed by an updown upload request
         self.mock_get_upload_data.assert_called_once_with(
-            self.app_name, self.app_version)
+            self.app_name, self.app_version, self.app_changelog,
+            self.app_archs)
         self.mock_post.assert_called_with(
             CLICK_UPDOWN_UPLOAD_URL,
             files={'binary': self.mock_open.return_value},
@@ -323,54 +347,35 @@
             'Click Package upload finished successfully.')
 
     def test_take_action_handles_error_while_getting_upload_data(self):
-        parsed_args = namedtuple('parsed_args',
-                                 'application_name, version, filename')
-
-        filename = 'package.click'
-        args = parsed_args(self.app_name, self.app_version, filename)
-
         response = Response()
         response.status_code = 200
         self.mock_post.return_value = response
         self.mock_get_upload_data.return_value = {
             'success': False, 'message': 'Some error'}
 
-        self.command.take_action(args)
+        self.command.take_action(self.args)
 
         self.mock_log.error.assert_called_with(
             'Error verifying upload request. Uploading not possible.\n'
             'Reason: %s', 'Some error')
 
     def test_take_action_handles_error_while_uploading(self):
-        parsed_args = namedtuple('parsed_args',
-                                 'application_name, version, filename')
-
-        filename = 'package.click'
-        args = parsed_args(self.app_name, self.app_version, filename)
-
         self.mock_post.side_effect = Exception('foo')
 
-        self.command.take_action(args)
+        self.command.take_action(self.args)
 
         self.mock_log.exception.assert_called_with(
                 'There was an error uploading the click package.')
 
     def test_take_action_handles_unsuccessful_response(self):
-        parsed_args = namedtuple('parsed_args',
-                                 'application_name, version, filename')
-
-        filename = 'package.click'
-        args = parsed_args(self.app_name, self.app_version, filename)
-
-        upload_response = {'success': True,
-                                  'message': None}
+        upload_response = {'success': True, 'message': None}
         response = Response()
         response.status_code = 200
         response.reason = 'OK'
         response._content = json.dumps(upload_response).encode('utf-8')
         self.mock_post.return_value = response
 
-        self.command.take_action(args)
+        self.command.take_action(self.args)
 
         self.mock_log.error.assert_called_with(
             'There was an error uploading the click package.\n'
@@ -379,19 +384,13 @@
             'OK', json.dumps(upload_response))
 
     def test_take_action_handles_error_response(self):
-        parsed_args = namedtuple('parsed_args',
-                                 'application_name, version, filename')
-
-        filename = 'package.click'
-        args = parsed_args(self.app_name, self.app_version, filename)
-
         response = Response()
         response.status_code = 500
         response.reason = 'INTERNAL SERVER ERROR'
         response._content = 'This is a traceback.'.encode('utf-8')
         self.mock_post.return_value = response
 
-        self.command.take_action(args)
+        self.command.take_action(self.args)
 
         self.mock_log.error.assert_called_with(
             'There was an error uploading the click package.\n'
@@ -400,12 +399,6 @@
             'INTERNAL SERVER ERROR', 'This is a traceback.')
 
     def test_take_action_handles_unsuccessful_mark_uploaded_response(self):
-        parsed_args = namedtuple('parsed_args',
-                                 'application_name, version, filename')
-
-        filename = 'package.click'
-        args = parsed_args(self.app_name, self.app_version, filename)
-
         upload_response = {'successful': True, 'message': None}
         response = Response()
         response.status_code = 200
@@ -416,7 +409,7 @@
         with patch('click.upload.Upload.mark_uploaded') as mock_mark_uploaded:
             mock_mark_uploaded.return_value = {
                 'success': False, 'message': 'Please try again later'}
-            self.command.take_action(args)
+            self.command.take_action(self.args)
 
         self.mock_log.error.assert_called_with(
             'There was an error after uploading the click package.\n'

=== modified file 'click/upload.py'
--- click/upload.py	2013-09-13 13:32:25 +0000
+++ click/upload.py	2013-10-09 14:25:04 +0000
@@ -23,9 +23,13 @@
 
     def get_parser(self, prog_name):
         parser = super(Upload, self).get_parser(prog_name)
-        parser.add_argument('application_name', nargs='?')
-        parser.add_argument('version', nargs='?')
-        parser.add_argument('filename', nargs='?')
+        parser.add_argument('application_name')
+        parser.add_argument('version')
+        parser.add_argument('filename')
+        parser.add_argument('changelog')
+        parser.add_argument(
+            'arch', nargs='*', default='all',
+            choices=['armhf', 'i386', 'amd64', 'all'])
         return parser
 
     def get_config(self):
@@ -58,7 +62,7 @@
             session = None
         return session
 
-    def get_upload_data(self, name, version):
+    def get_upload_data(self, name, version, changelog, architectures):
         myapps_url = os.environ.get('MYAPPS_API_ROOT_URL', MYAPPS_API_ROOT_URL)
         myapps_url = urljoin(myapps_url, 'click-upload/%s/' % name)
 
@@ -70,7 +74,10 @@
             return result
 
         try:
-            response = session.post(myapps_url, data={'version': version})
+            response = session.post(
+                myapps_url,
+                data={'version': version, 'changelog': changelog,
+                      'architectures': architectures})
             if response.ok:
                 result.update(response.json())
                 message = result.get('message')
@@ -118,8 +125,11 @@
         application_name = parsed_args.application_name
         version = parsed_args.version
         filename = parsed_args.filename
+        changelog = parsed_args.changelog
+        architectures = parsed_args.arch
 
-        data = self.get_upload_data(application_name, version)
+        data = self.get_upload_data(
+            application_name, version, changelog, architectures)
         if not data.get('success', False):
             error = data.get('message', 'Unknown error')
             self.log.error(