5
\$\begingroup\$

I wrote this simple API to be able to send logs from the client to MongoDB. Even though this works and fits my need, I would like to point out some concerns that I have regarding the following:

  1. Is it efficient to be calling the instance of MongoClient for every request?
  2. Do I need to close the instance of MongoClient after the request is successful?
  3. A more Pythonic way to return responses?
from flask import Flask, jsonify, request, Response
from flask_restful import Resource, Api
from pymongo import MongoClient
import json


app = Flask(__name__)
api = Api(app)


USER = "user"
PASS = "pw"
MONGO_URI = 'mongodb://%s:%s@test.mlab.com/test' % (USER, PASS)
PORT = 19788


def db_conn():
    client = MongoClient(MONGO_URI, PORT)
    return client

def insert_record(args):
    client = db_conn()
    replycode = 0
    try:
        db = client['test']
        posts = db.users
        posts.insert(args)
    except:
        replycode = 1

    return replycode

def select_record(args={}):

    client = db_conn()
    db = client['test']
    result = db.users.find(args)
    return result

class CreatUser(Resource):

    def post(self):
        try:
            content = request.get_json()
            if "Vehicle" not in content:
                return jsonify({"Result": "Vehicle number not in passed arguments"}), 400

            else:
                vehicle = content['Vehicle']

            reply = insert_record(content)
            if reply == 0:
                return jsonify({"Result" : "Successfully inserted user: " + vehicle}), 201
            else:
                return jsonify({"Result" : "Failed to insert data. Check logs for more details"}), 400

        except Exception as e:
            return jsonify({'Error' : str(e)}), 500

class ViewUser(Resource):
    def get(self):
        from bson import json_util

        results = select_record()
        final = []
        for result in results:
            result.pop("_id")
            final.append(result)

        return jsonify(results=final), 200

api.add_resource(CreatUser, "/api/create")
api.add_resource(ViewUser, "/api/view")

if __name__ == "__main__":
    app.run(debug=True)
\$\endgroup\$
2
+50
\$\begingroup\$

Ad. 1 & 2:

There is no need to open fresh connection for, and close it after request. MongoClient has connection-pooling built in, so only thing you need to do is to create instance of MongoClient with connection parameters and optional pool size. It will open connection on first usage and in case it gets closed or times out - reopen them when needed.

A good way to do, would be to build a plugin wrapping MongoClient like they did with SQLite in this example. Such plugin can be used afterwards as persistent connection - all logic regarding connecting/keeping/reconnecting will happen inside, thus simplifying resource methods. But it is not necessary, instance of MongoClient can be created in global scope and imported into functions in same way like you did with db_conn().

Ad. 3:

IMHO there is nothing wrong with jsonify since Flask-RESTful recognises Flasks response objects. But, as they show in here you don't need to do it and you can return dicts directly:

class Todo3(Resource):
    def get(self):
        # Set the response code to 201 and return custom headers
        return {'task': 'Hello world'}, 201, {'Etag': 'some-opaque-string'}

I would also recommend putting USER, PASS, MONGO_URI, PORT into configuration file, so they can be easily overridden by environment variables on production.

Also Artyom24 pointed out nice improvements regarding API RESTfulness

\$\endgroup\$
  • \$\begingroup\$ thank you for the suggestion, but is it really necessary to wrap the MongoClient? Or it just depends on how complicated the project gets? \$\endgroup\$ – wasp8898 Jan 16 '17 at 7:33
  • \$\begingroup\$ no, it is not mandatory, you can create MongoClient instance in global scope and then use it in functions. Updated my answer to include this for completeness. \$\endgroup\$ – potfur Jan 16 '17 at 8:43
3
\$\begingroup\$

The MongoClient was designed to be instantiated once per app life cycle, so this is redundant. You should try to reuse MongoClient instances as much as possible. Basically the only reason to create a new instance is if you want to configure it in a different way.

Also you've mentioned your API is supposed to be RESTful, so a few notes here:

1) To deal with user resource you should include it in the uri in pluralize form and distinguish actions you are performing by standard http codes, like the following:

/api/users POST - to create a new user

/api/users/{userId} GET - to fetch an existing one by userId

2) when you a new user is created it is a good practise to return 201 (Created) http code together with the full url to fetch the user which has been just created, like: http://yourdomain.com/api/users/{userId}

3) "Failed to insert data. Check logs for more details" - it doesn't look like a 400 Bad Request, as the user didn't make any mistake with input params. I suppose it must be either 500 (if the issue is related to something like a database error for example) or 403 Forbidden if any of implicit domain rules has been violated

Hope this helps

\$\endgroup\$
  • \$\begingroup\$ thank you for your suggestions, i will try to apply your notes on my next iteration! :) \$\endgroup\$ – wasp8898 Jan 17 '17 at 0:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.