1
\$\begingroup\$

The tutorial is here

I want to build a RestFul API with Flask to query User Agent data. Curl script shows below. The api should work with three or two or one or even zero argument.

So how to change the code to make it more simple and readable?

curl -i http://localhost:5000/todo/api/v1.0/tasks -u miguel:python -i -H "Content-Type: application/json" -X POST -d '{"os_family":"iOS","device_brand":"Apple"}'
None: return default data
One: {"os_family":"iOS"}
Two : {"os_family":"iOS","device_brand":"Apple"}, or {"os_family":"iOS","browser":"Mobile Safari"}, or {"device_brand":"Apple","browser":"Mobile Safari"}
Three :{"device_brand":"Apple","browser":"Mobile Safari","device_brand":"Apple"}

Source code

#!flask/bin/python
"""Alternative version of the ToDo RESTful server implemented using the
Flask-RESTful extension."""
from flask import Flask, jsonify, abort, make_response
from flask_restful import Api, Resource, reqparse, fields, marshal
from flask_httpauth import HTTPBasicAuth
from pymongo import MongoClient
import random
client = MongoClient('10.211.55.12', 27018)
db = client.useragent
post = db.ua_final
app = Flask(__name__, static_url_path="")
api = Api(app)
auth = HTTPBasicAuth()
import time
import datetime
@auth.get_password
def get_password(username):
 if username == 'miguel':
 return 'python'
 return None
@auth.error_handler
def unauthorized():
 # return 403 instead of 401 to prevent browsers from displaying the default
 # auth dialog
 return make_response(jsonify({'message': 'Unauthorized access'}), 403)
tasks = [
 {
 'id': 1,
 'ua': u"Mozilla/5.0 (iPhone; CPU iPhone OS 5_1 like Mac OS X) AppleWebKit/534.46 (KHTML, like Gecko) Version/5.1 Mobile/9B179 Safari/7534.48.3",
 'done': False
 },
 {
 'id': 2,
 'ua': u"Mozilla/5.0 (Linux; U; Android 4.0.4; en-gb; GT-I9300 Build/IMM76D) AppleWebKit/534.30 (KHTML, like Gecko) Version/4.0 Mobile Safari/534.30",
 'done': False
 }
]
task_fields = {
 'ua': fields.String,
 'done': fields.Boolean,
 'uri': fields.Url('task')
}
class TaskListAPI(Resource):
 decorators = [auth.login_required]
 def __init__(self):
 self.reqparse = reqparse.RequestParser()
 self.reqparse.add_argument('ua', type=str, required=True,
 help='No ua provided',
 location='json')
 self.reqparse.add_argument('description', type=str, default="",
 location='json')
 super(TaskListAPI, self).__init__()
 def get(self):
 return {'tasks': [marshal(task, task_fields) for task in tasks]}
class TaskAPI(Resource):
 decorators = [auth.login_required]
 def __init__(self):
 self.reqparese = reqparse.RequestParser()
 self.reqparese.add_argument(
 'number', type=int, default=10, location='json')
 self.reqparese.add_argument(
 'browser', type=str, default='', location='json')
 self.reqparese.add_argument(
 'os_family', type=str, default='', location='json')
 self.reqparese.add_argument(
 'device_brand', type=str, default='', location='json')
 super(TaskAPI, self).__init__()
 def delete_id(self, newDict):
 del newDict['_id']
 return newDict['UA']
 def post(self):
 args = self.reqparese.parse_args()
 # print args
 if args['browser'] == '' and args['os_family'] == '' and args['device_brand'] == '':
 return {'results': random.sample([self.delete_id(i) for i in post.find().limit(1000)], 10)}
 if args['browser'] == '' or args['os_family'] == '' or args['device_brand'] == '':
 if len([self.delete_id(i) for i in post.find({'device_brand': args['device_brand'], 'os_family':args['os_family'], 'browser_family':args['browser']}).limit(1000)]) != 0:
 return {'results': random.sample([self.delete_id(i) for i in post.find({"$or": [{'os_family': args['os_family']}, {'browser_family': args['browser']}]}).limit(1000)], 10)}
 else:
 return {'results': None}
 if args['browser'] != '' and args['os_family'] != '' and args['device_brand'] != '':
 if len([self.delete_id(i) for i in post.find({'device_brand': args['device_brand'], 'os_family':args['os_family'], 'browser_family':args['browser']}).limit(1000)]) != 0:
 return {'results': random.sample([self.delete_id(i) for i in post.find({'device_brand': args['device_brand'], 'os_family':args['os_family'], 'browser_family':args['browser']}).limit(1000)], 10)}
 else:
 return {'results': None}
 # result = [i del i['_id'] for i in post.find(limit=10)]
 # return {'results': random.sample(result,1)}
api.add_resource(TaskListAPI, '/todo/api/v1.0/tasks', endpoint='tasks')
api.add_resource(TaskAPI, '/todo/api/v1.0/tasks', endpoint='task')
if __name__ == '__main__':
 app.run(host='0.0.0.0', port=5000, debug=True)

Source data

/* 1 */
{
 "_id" : "3e55d425bf7e8bd95be302fb76e47371",
 "device_family" : "Other",
 "device_model" : null,
 "browser_version_string" : "",
 "os_version_string" : "",
 "os_version" : [],
 "browser_version" : [],
 "os_family" : "Mac OS X",
 "device_brand" : null,
 "UA" : "Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/419 (KHTML, like Gecko, Safari/419.3) Cheshire/1.0.ALPHA",
 "browser_family" : "Safari"
}
/* 2 */
{
 "_id" : "64b0d163d57ea36f0a8bf0a827e6d94f",
 "device_family" : "Other",
 "device_model" : null,
 "browser_version_string" : "",
 "os_version_string" : "",
 "os_version" : [],
 "browser_version" : [],
 "os_family" : "Mac OS X",
 "device_brand" : null,
 "UA" : "Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/418.9 (KHTML, like Gecko, Safari/111) Cheshire/1.0.ALPHA",
 "browser_family" : "Safari"
}
/* 3 */
{
 "_id" : "b0de66dc426faf27832b6649d39450b7",
 "device_family" : "Other",
 "device_model" : null,
 "browser_version_string" : "",
 "os_version_string" : "",
 "os_version" : [],
 "browser_version" : [],
 "os_family" : "Mac OS X",
 "device_brand" : null,
 "UA" : "Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/418.9 (KHTML, like Gecko, Safari) Safari/419.3 Cheshire/1.0.ALPHA",
 "browser_family" : "Safari"
}
/* 4 */
{
 "_id" : "bc4915d3c02ba508cbeb5d3a2584fb87",
 "device_family" : "Other",
 "device_model" : null,
 "browser_version_string" : "",
 "os_version_string" : "",
 "os_version" : [],
 "browser_version" : [],
 "os_family" : "Mac OS X",
 "device_brand" : null,
 "UA" : "Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/418.9 (KHTML, like Gecko) Safari/419.3 Cheshire/1.0.ALPHA",
 "browser_family" : "Safari"
}
/* 5 */
{
 "_id" : "b08637a6eb4a57310d9105d527fee5ba",
 "device_family" : "Other",
 "device_model" : null,
 "browser_version_string" : "",
 "os_version_string" : "",
 "os_version" : [],
 "browser_version" : [],
 "os_family" : "Mac OS X",
 "device_brand" : null,
 "UA" : "Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en) AppleWebKit/419 (KHTML, like Gecko, Safari/419.3) Cheshire/1.0.ALPHA",
 "browser_family" : "Safari"
}
/* 6 */
{
 "_id" : "b489e2d84b8f6543e2ebfe36700ae023",
 "device_family" : "Other",
 "device_model" : null,
 "browser_version_string" : "",
 "os_version_string" : "",
 "os_version" : [],
 "browser_version" : [],
 "os_family" : "Mac OS X",
 "device_brand" : null,
 "UA" : "Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en) AppleWebKit/419 (KHTML, like Gecko, Safari/125) Cheshire/1.0.ALPHA",
 "browser_family" : "Safari"
}
/* 7 */
{
 "_id" : "069aaf22239724549b9c21711e3d69a2",
 "device_family" : "Other",
 "device_model" : null,
 "browser_version_string" : "7.0.3",
 "os_version_string" : "10.9.3",
 "os_version" : [ 
 10, 
 9, 
 3
 ],
 "browser_version" : [ 
 7, 
 0, 
 3
 ],
 "os_family" : "Mac OS X",
 "device_brand" : null,
 "UA" : "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_3) AppleWebKit/537.75.14 (KHTML, like Gecko) Version/7.0.3 Safari/7046A194A",
 "browser_family" : "Safari"
}
/* 8 */
{
 "_id" : "c15ec0412f029f692ec01fae06c68b53",
 "device_family" : "iPad",
 "device_model" : "iPad",
 "browser_version_string" : "6.0",
 "os_version_string" : "6.0",
 "os_version" : [ 
 6, 
 0
 ],
 "browser_version" : [ 
 6, 
 0
 ],
 "os_family" : "iOS",
 "device_brand" : "Apple",
 "UA" : "Mozilla/5.0 (iPad; CPU OS 6_0 like Mac OS X) AppleWebKit/536.26 (KHTML, like Gecko) Version/6.0 Mobile/10A5355d Safari/8536.25",
 "browser_family" : "Mobile Safari"
}
/* 9 */
{
 "_id" : "5d265f6e2bc2aa0905a72770ce4dc3ea",
 "device_family" : "Other",
 "device_model" : null,
 "browser_version_string" : "5.1.3",
 "os_version_string" : "10.7.3",
 "os_version" : [ 
 10, 
 7, 
 3
 ],
 "browser_version" : [ 
 5, 
 1, 
 3
 ],
 "os_family" : "Mac OS X",
 "device_brand" : null,
 "UA" : "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_3) AppleWebKit/534.55.3 (KHTML, like Gecko) Version/5.1.3 Safari/534.53.10",
 "browser_family" : "Safari"
}
/* 10 */
{
 "_id" : "25740aba95599f5ea63181821f0d56d7",
 "device_family" : "iPad",
 "device_model" : "iPad",
 "browser_version_string" : "5.1",
 "os_version_string" : "5.1",
 "os_version" : [ 
 5, 
 1
 ],
 "browser_version" : [ 
 5, 
 1
 ],
 "os_family" : "iOS",
 "device_brand" : "Apple",
 "UA" : "Mozilla/5.0 (iPad; CPU OS 5_1 like Mac OS X) AppleWebKit/534.46 (KHTML, like Gecko ) Version/5.1 Mobile/9B176 Safari/7534.48.3",
 "browser_family" : "Mobile Safari"
}
Heslacher
50.8k5 gold badges83 silver badges177 bronze badges
asked Mar 28, 2017 at 12:33
\$\endgroup\$
2
  • \$\begingroup\$ I have rolled back the last edit. Please see what you may and may not do after receiving answers . \$\endgroup\$ Commented Mar 29, 2017 at 4:42
  • \$\begingroup\$ The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review - Asking Questions for guidance on writing good question titles. \$\endgroup\$ Commented Mar 29, 2017 at 9:06

1 Answer 1

1
\$\begingroup\$

If you run flake8 you will get following remarks:

test.py:6:1: F401 'flask.abort' imported but unused
test.py:20:1: E402 module level import not at top of file
test.py:20:1: F401 'time' imported but unused
test.py:21:1: E402 module level import not at top of file
test.py:21:1: F401 'datetime' imported but unused
test.py:37:1: E305 expected 2 blank lines after class or function definition, found 1
test.py:40:80: E501 line too long (152 > 79 characters)
test.py:45:80: E501 line too long (157 > 79 characters)
test.py:95:80: E501 line too long (92 > 79 characters)
test.py:96:80: E501 line too long (103 > 79 characters)
test.py:98:80: E501 line too long (90 > 79 characters)
test.py:99:80: E501 line too long (181 > 79 characters)
test.py:100:80: E501 line too long (187 > 79 characters)
test.py:103:80: E501 line too long (92 > 79 characters)
test.py:104:80: E501 line too long (181 > 79 characters)
test.py:105:80: E501 line too long (210 > 79 characters)

These inherently long lines are especially bad.

If you take a look at flask_restful.reqparse you'll discover is marked as obsolete and it will be removed in the future. I'd advise to use Cerberus library to authorize, coerce and parse passed JSON data.

There's no reason to make task_fields global. You should probably move it to TaskListAPI class, since it's the only one using it, and capitalize it, since it's mean to be used as contanst:

class TaskListAPI(Resource):
 TASK_FIELDS = ..

This code with multiple and is rather bad looking:

if args['browser'] == '' and args['os_family'] == '' and args['device_brand'] == ''

Can be rewritten as follows:

if not any([args['browser'], args['os_family'], args['device_brand'])

This complicated logic inside TaskAPI.post should be rewritten using temporary variables. As flake8 reports, if there's more than 80 characters in a line, you need to rewrite it.

answered Mar 28, 2017 at 15:52
\$\endgroup\$
1
  • \$\begingroup\$ Great Thanks, Updated as your suggestion except 'Cerberus', I will use it in later project. Is that ok now? \$\endgroup\$ Commented Mar 29, 2017 at 3:54

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.