From f5854303998a04ce19b6d1d0586fc838d2e4bc70 Mon Sep 17 00:00:00 2001 From: yackob03 Date: Fri, 24 Jan 2014 17:00:42 -0500 Subject: [PATCH] Make abort return a json block, which is apparently what the client expects. Remove unused imports. Fix line length and kwarg problems. --- endpoints/api.py | 47 ++++++++++++++++++++++++------------------- endpoints/index.py | 6 +++--- endpoints/registry.py | 22 ++++++++++++-------- util/http.py | 22 +++++++++++++------- 4 files changed, 58 insertions(+), 39 deletions(-) diff --git a/endpoints/api.py b/endpoints/api.py index 90b0140d9..273736910 100644 --- a/endpoints/api.py +++ b/endpoints/api.py @@ -72,10 +72,10 @@ def endpoint_invalid_request(e): }) -def request_error(exception = None, **kwargs): +def request_error(exception=None, **kwargs): data = kwargs.copy() if exception: - data['message'] = ex.message + data['message'] = exception.message return make_response(jsonify(data), 400) @@ -254,12 +254,14 @@ def convert_user_to_organization(): # Ensure that the new admin user is the not user being converted. admin_username = convert_data['adminUser'] if admin_username == user.username: - return request_error(reason = 'invaliduser', message = 'The admin user is not valid') + return request_error(reason='invaliduser', + message='The admin user is not valid') # Ensure that the sign in credentials work. admin_password = convert_data['adminPassword'] if not model.verify_user(admin_username, admin_password): - return request_error(reason = 'invaliduser', message = 'The admin user credentials are not valid') + return request_error(reason='invaliduser', + message='The admin user credentials are not valid') # Subscribe the organization to the new plan. plan = convert_data['plan'] @@ -295,14 +297,15 @@ def change_user_details(): new_email = user_data['email'] if model.find_user_by_email(new_email): # Email already used. - return request_error(message = 'E-mail address already used') + return request_error(message='E-mail address already used') - logger.debug('Sending email to change email address for user: %s', user.username) + logger.debug('Sending email to change email address for user: %s', + user.username) code = model.create_confirm_email_code(user, new_email=new_email) send_change_email(user.username, user_data['email'], code.code) except model.InvalidPasswordException, ex: - return request_error(exception = ex) + return request_error(exception=ex) return jsonify(user_view(user)) @@ -314,7 +317,7 @@ def create_new_user(): existing_user = model.get_user(user_data['username']) if existing_user: - return request_error(message = 'The username already exists') + return request_error(message='The username already exists') try: new_user = model.create_user(user_data['username'], user_data['password'], @@ -323,7 +326,7 @@ def create_new_user(): send_confirmation_email(new_user.username, new_user.email, code.code) return make_response('Created', 201) except model.DataModelException as ex: - return request_error(exception = ex) + return request_error(exception=ex) @api.route('/signin', methods=['POST']) @@ -467,14 +470,15 @@ def create_organization(): pass if existing: - return request_error(message = 'A user or organization with this name already exists') + msg = 'A user or organization with this name already exists' + return request_error(message=msg) try: model.create_organization(org_data['name'], org_data['email'], current_user.db_user()) return make_response('Created', 201) except model.DataModelException as ex: - return request_error(exception = ex) + return request_error(exception=ex) def org_view(o, teams): @@ -529,7 +533,7 @@ def change_organization_details(orgname): if 'email' in org_data and org_data['email'] != org.email: new_email = org_data['email'] if model.find_user_by_email(new_email): - return request_error(message = 'E-mail address already used') + return request_error(message='E-mail address already used') logger.debug('Changing email address for organization: %s', org.username) model.update_email(org, new_email) @@ -614,7 +618,8 @@ def create_organization_prototype_permission(orgname): details = request.get_json() activating_username = None - if 'activating_user' in details and details['activating_user'] and 'name' in details['activating_user']: + if ('activating_user' in details and details['activating_user'] and + 'name' in details['activating_user']): activating_username = details['activating_user']['name'] delegate = details['delegate'] @@ -632,10 +637,10 @@ def create_organization_prototype_permission(orgname): if delegate_teamname else None) if activating_username and not activating_user: - return request_error(message = 'Unknown activating user') + return request_error(message='Unknown activating user') if not delegate_user and not delegate_team: - return request_error(message = 'Missing delagate user or team') + return request_error(message='Missing delagate user or team') role_name = details['role'] @@ -893,7 +898,7 @@ def update_organization_team_member(orgname, teamname, membername): # Find the user. user = model.get_user(membername) if not user: - return request_error(message = 'Unknown user') + return request_error(message='Unknown user') # Add the user to the team. model.add_user_to_team(user, team) @@ -934,7 +939,7 @@ def create_repo(): existing = model.get_repository(namespace_name, repository_name) if existing: - return request_error(message = 'Repository already exists') + return request_error(message='Repository already exists') visibility = req['visibility'] @@ -1007,7 +1012,7 @@ def list_repos(): if page: try: page = int(page) - except: + except Exception: page = None username = None @@ -1537,7 +1542,7 @@ def change_user_permissions(namespace, repository, username): # This repository is not part of an organization pass except model.DataModelException as ex: - return request_error(exception = ex) + return request_error(exception=ex) log_action('change_repo_permission', namespace, {'username': username, 'repo': repository, @@ -1590,7 +1595,7 @@ def delete_user_permissions(namespace, repository, username): try: model.delete_user_permission(username, namespace, repository) except model.DataModelException as ex: - return request_error(exception = ex) + return request_error(exception=ex) log_action('delete_repo_permission', namespace, {'username': username, 'repo': repository}, @@ -1846,7 +1851,7 @@ def subscribe(user, plan, token, require_business_plan): plan_found['price'] == 0): logger.warning('Business attempting to subscribe to personal plan: %s', user.username) - return request_error(message = 'No matching plan found') + return request_error(message='No matching plan found') private_repos = model.get_private_repo_count(user.username) diff --git a/endpoints/index.py b/endpoints/index.py index 9cb6f61f5..b740bf998 100644 --- a/endpoints/index.py +++ b/endpoints/index.py @@ -2,12 +2,12 @@ import json import logging import urlparse -from flask import request, make_response, jsonify, abort as flask_abort, session, Blueprint +from flask import request, make_response, jsonify, session, Blueprint from functools import wraps from data import model from data.queue import webhook_queue -from app import app, mixpanel +from app import mixpanel from auth.auth import (process_auth, get_authenticated_user, get_validated_token) from util.names import parse_repository_name @@ -15,9 +15,9 @@ from util.email import send_confirmation_email from auth.permissions import (ModifyRepositoryPermission, UserPermission, ReadRepositoryPermission, CreateRepositoryPermission) - from util.http import abort + logger = logging.getLogger(__name__) index = Blueprint('index', __name__) diff --git a/endpoints/registry.py b/endpoints/registry.py index 3218e38bf..4d02dfd14 100644 --- a/endpoints/registry.py +++ b/endpoints/registry.py @@ -1,8 +1,8 @@ import logging import json -from flask import (make_response, request, session, Response, abort as flask_abort, - redirect, Blueprint) +from flask import (make_response, request, session, Response, redirect, + Blueprint) from functools import wraps from datetime import datetime from time import time @@ -59,7 +59,8 @@ def require_completion(f): def wrapper(namespace, repository, *args, **kwargs): if store.exists(store.image_mark_path(namespace, repository, kwargs['image_id'])): - abort(400, 'Image %(image_id)s is being uploaded, retry later', image_id=kwargs['image_id']) + abort(400, 'Image %(image_id)s is being uploaded, retry later', + image_id=kwargs['image_id']) return f(namespace, repository, *args, **kwargs) return wrapper @@ -194,7 +195,8 @@ def put_image_checksum(namespace, repository, image_id): abort(400, "Missing checksum for image %(image_id)s", image_id=image_id) if not session.get('checksum'): - abort(400, 'Checksum not found in Cookie for image %(imaage_id)s', image_id=image_id) + abort(400, 'Checksum not found in Cookie for image %(imaage_id)s', + image_id=image_id) if not store.exists(store.image_json_path(namespace, repository, image_id)): abort(404, 'Image not found: %(image_id)s', image_id=image_id) @@ -321,10 +323,12 @@ def put_image_json(namespace, repository, image_id): except json.JSONDecodeError: pass if not data or not isinstance(data, dict): - abort(400, 'Invalid JSON for image: %(image_id)s\nJSON: %(json)s', image_id=image_id, json=request.data) + abort(400, 'Invalid JSON for image: %(image_id)s\nJSON: %(json)s', + image_id=image_id, json=request.data) if 'id' not in data: - abort(400, 'Missing key `id` in JSON for image: %(image_id)s', image_id=image_id) + abort(400, 'Missing key `id` in JSON for image: %(image_id)s', + image_id=image_id) # Read the checksum checksum = request.headers.get('X-Docker-Checksum') @@ -338,10 +342,12 @@ def put_image_json(namespace, repository, image_id): # We cleanup any old checksum in case it's a retry after a fail store.remove(store.image_checksum_path(namespace, repository, image_id)) if image_id != data['id']: - abort(400, 'JSON data contains invalid id for image: %(image_id)s', image_id=image_id) + abort(400, 'JSON data contains invalid id for image: %(image_id)s', + image_id=image_id) parent_id = data.get('parent') - if parent_id and not store.exists(store.image_json_path(namespace, repository, parent_id)): + if (parent_id and not + store.exists(store.image_json_path(namespace, repository, parent_id))): abort(400, 'Image %(image_id)s depends on non existing parent image %(parent_id)s', image_id=image_id, parent_id=parent_id) diff --git a/util/http.py b/util/http.py index 05f27970e..2f8c50236 100644 --- a/util/http.py +++ b/util/http.py @@ -1,10 +1,12 @@ import logging -from flask import request, abort as flask_abort, make_response -from auth.auth import process_auth, extract_namespace_repo_from_session, get_authenticated_user, get_validated_token +from flask import request, abort as flask_abort, jsonify +from auth.auth import get_authenticated_user, get_validated_token + logger = logging.getLogger(__name__) + DEFAULT_MESSAGE = {} DEFAULT_MESSAGE[400] = 'Invalid Request' DEFAULT_MESSAGE[401] = 'Unauthorized' @@ -13,7 +15,8 @@ DEFAULT_MESSAGE[404] = 'Not Found' DEFAULT_MESSAGE[409] = 'Conflict' def abort(status_code, message=None, **kwargs): - message = str(message) % kwargs if message else DEFAULT_MESSAGE.get(status_code, '') + message = (str(message) % kwargs if message else + DEFAULT_MESSAGE.get(status_code, '')) # Add the user information. auth_user = get_authenticated_user() @@ -21,11 +24,16 @@ def abort(status_code, message=None, **kwargs): if auth_user: message = '%s (user: %s)' % (message, auth_user.username) elif auth_token: - message = '%s (token: %s)' % (message, auth_token.friendly_name or auth_token.code) + message = '%s (token: %s)' % (message, + auth_token.friendly_name or auth_token.code) # Log the abort. - logger.error('Error %s: %s. Arguments: %s' % (status_code, message, request.view_args)) + logger.warning('Error %s: %s. Arguments: %s' % (status_code, message, + request.view_args)) + + resp = jsonify({'error': message}) + resp.status_code = status_code # Report the abort to the user. - flask_abort(make_response(message, status_code, {})) - + + flask_abort(resp)