Flask m’a tuer

Flask m’a tuer

Vous le savez peut-être déjà, je suis un peu extrémiste.

Je refuse de reconnaître qu’un outil est bon simplement parce qu’il permet de faire le job. Et je me méfie comme de la peste des outils qui permettent de faire le job facilement.

Flask, le framework Python, n’a à ces égards pas de chance, car tout le monde le décrit comme étant diaboliquement facile d’usage, et violemment efficace. Assez à la mode depuis un moment (on en parle sur une bonne moitié des articles du flux RSS de Python Weekly), ce n’était qu’une affaire de temps avant que je le vois débarquer dans un des POCs que je dois durcir.
Évidemment c’est rapidement arrivé; et franchement, si on veut faire du développement un minimum sérieux, ce framework donne envie de rouler au Caterpillar sur des paniers de chatons mignons.

Le contexte tout d’abord

Prenons une application utilisant Flask comme base afin de servir du bon vieux HTML d’un côté, et une api REST de l’autre. Histoire de rajouter du délire pétillant dans la marmite, on va aussi accéder à la base via Flask (flask.ext.pymongo).
Le tout est dans un bon vieux virtualenv des familles (oui, cette information n’apporte et n’apportera rien, c’est juste pour imprégner dans votre esprit que virtualenv, malgré ses nombreux défauts, c’est bien).

Premier code

Soit un code trivial produit par un pauvre besogneux noob de Flask qui a fervemment suivit les tutos du ouaibe afin de répondre aux différents « besoins » précisés ci-avant, dont l’archi et le code viennent ci-après (baladez votre curseur sur le code pour voir le nom des fichiers) :

run.py
pack/
 |--- __init__.py
 |--- models.py
 |--- views.py
 |--- api.py
test/
 |--- tests.py
#!/usr/bin/env python2.7

from pack import app
if __name__ == "__main__":
    app.run()
from flask import Flask
import flask.ext.pymongo as pymongo

app = Flask(__name__)
app.config["MONGO_DBNAME"] = "my_database"
mongo = pymongo.PyMongo(app)

import views
import api
from pack import mongo

def new_object(string):
    number = mongo.db.my_collection.count()
    mongo.db.my_collection.insert({"string": string, "num": number})
from pack import app
from pack import mongo

@app.route("/")
def view():
    num = mongo.db.my_collection.count()
    return "<h1>Oh boy!</h1>You got %s tasks on yo base!" % num

@app.route("/tasks")
def tasks():
    tasks = mongo.db.my_collection.find({}, {"_id": False})
    string = "<table><tr><th>Number</th><th>String</th>"
    for task in tasks:
        string += "<tr><td>%s</td><td>%s</td></tr>" % (task["num"], task["string"])
    return string + "</table>"
from flask import jsonify
from pack import app
from pack import mongo
from pack.models import new_object

@app.route("/api/info/")
def show():
    result = {"result": list(mongo.db.my_collection.find({}, {"_id": False}))}
    return jsonify(result)

@app.route("/api/insert/<string:ma_string>")
def insert(ma_string):
    new_object(ma_string)
    return jsonify({"msg": "ok"})

Analyse statique

 C’est foutrement moche.

Pas la moindre classe, du mélange immonde de langages aux finalités différentes (code Python et affichage HTML – voir views.py), des globales en veux-tu en voilà, des décorateurs qu’on ne maîtrise absolument pas, des imports louches (pour définir les routes après l’instanciation de Flask)… tout ce qu’il faut pour garantir un programme bien figé et non viable après 6 mois de développement.

Je trouve aussi assez douteux que l’app soit instanciée dans le __init__.py (et donc à chaud lorsqu’on importe le package). Pour moi ce devrait être une ligne présente dans le run.py (et in fine dans uwsgi, qui est assez malin pour instancier une classe )… Au lieu de ça on manipule (à nouveau) une belle globale app toute moche qui s’est instanciée dans notre dos sans qu’on ai rien demandé (et sera instanciée à chaque fois qu’on importera le package depuis l’extérieur… ce qui arrive à chaque fichier de test !). C’est pas net en terme d’architecture…

On n’a même pas encore commencé à fouiller et c’est déjà tellement sale que je n’oserai pas l’utiliser pour un skyblog. Essayons néanmoins d’en faire quelque chose.

Analyse moins statique

On commence par regarder la base, le fichier models.py. Et la base de la base : l’accès à la base de données.

On lance donc une console Python, histoire de voir les méthodes qu’on a sous la main et manipuler un peu l’outil…

In [1]: from flask import Flask
In [2]: from flask.ext.pymongo import PyMongo
In [3]: app = Flask("test")
In [4]: mongo = PyMongo(app)
In [8]: help(mongo.db)
---------------------------------------------------------------------------
RuntimeError Traceback (most recent call last)
<ipython-input-8-05680f0d501a> in <module>()
----> 1 help(mongo.db)

/usr/local/lib/python2.7/dist-packages/flask_pymongo/__init__.pyc in db(self)
255 parameter.
256 """
--> 257 if self.config_prefix not in current_app.extensions['pymongo']:
258 raise Exception('not initialized. did you forget to call init_app?')
259 return current_app.extensions['pymongo'][self.config_prefix][1]

/usr/local/lib/python2.7/dist-packages/werkzeug/local.pyc in __getattr__(self, name)
336 if name == '__members__':
337 return dir(self._get_current_object())
--> 338 return getattr(self._get_current_object(), name)
339
340 def __setitem__(self, key, value):

/usr/local/lib/python2.7/dist-packages/werkzeug/local.pyc in _get_current_object(self)
295 """
296 if not hasattr(self.__local, '__release_local__'):
--> 297 return self.__local()
298 try:
299 return getattr(self.__local, self.__name__)

/usr/local/lib/python2.7/dist-packages/flask/globals.pyc in _find_app()
32 top = _app_ctx_stack.top
33 if top is None:
---> 34 raise RuntimeError('working outside of application context')
35 return top.app
36

RuntimeError: working outside of application context

o_o Sérieusement ? Je dois mettre un contexte en place pour accéder à l’aide ?

Et voilà, en 5 lignes de codes, le pire point noir de Flask : tout ce qu’on manipule est construit sous la forme de wrapper / proxy totalement dépendant de l’instance de Flask et est totalement incapable de vivre sans elle.
Flask n’est pas le gentil berger qui donne accès aux pâtures à ses brebis (comprendre : met en forme puis distribue simplement des arguments aux bibliothèques qu’il intègre), Flask est le parasite qui asservit un animal viable et refuse de le voir exister en dehors de son strict contrôle (comprendre … mais vous avez compris).

Il faut donc créer un contexte – et donc générer un instance de Flask – pour, entre autre :

  • accéder à la base de données, alors que dans le cas présent, pymongo fait très bien le taff tout seul, le wrapper de Flask se contentant de rajouter 2 piètres méthodes de manipulations de fichiers faciles à remplacer par du code qu’on maîtrise;
  • faire un rendu HTML (c’est à dire qu’on associe un dictionnaire de valeur à un template HTML à travers la fonction flask.render_template) via Jinja2. Alors qu’encore une fois, dans ce cas Jinja2 se débrouille très bien tout seul;
  • générer une réponse contenant du json (flask.jsonify). Cette méthode construit une instance de flask.Response, une bête surcouche par dessus werkzeug.Reponse
  •  accéder aux données d’une requête via la globale flask.request, qui est un des pires poisons de Flask du fait de la difficulté à l’extraire d’un code l’utilisant.

Quand on regarde notre code, on se rend compte que chacun de nos fichiers est emprisonné dans Flask… Tester l’application efficacement, sans overhead énorme juste pour instancier Flask et les contextes ? Impossible. Réutiliser des portions de code pour d’autres projets (sans aucun rapport avec Flask) ? Impossible. Avoir ce sentiment de liberté que tout développeur Python devrait connaître ? Impossible.

Cette dépendance à notre instance Flask est d’autant plus vicieuse qu’elle est le meilleure moyen de créer des boucles de dépendances au niveau des imports. Typiquement, ce n’est pas un hasard si, dans __init__.py, les imports de views et api sont à la fin.

Tests

from bson.json_util import loads
from unittest import TestCase
from flask import Response

import pack.views as views
import pack.models as models
import pack.api as api
from pack import app

class test_models(TestCase):

    def tearDown(self):
        with app.test_request_context():
            for collection in [item for item in models.mongo.db.collection_names()
                               if not (item == "system.indexes")]:
                models.mongo.db.drop_collection(collection)

    def test001_new_object(self):
        with app.test_request_context():
            models.new_object("test")
            result = list(models.mongo.db.my_collection.find({}, {"_id": False}))
            self.assertEquals(len(result), 1)
            self.assertEquals(result[0], {"string": "test", "num": 0})

class test_views(TestCase):

    def tearDown(self):
        with app.test_request_context():
            for collection in [item for item in models.mongo.db.collection_names()
                               if not (item == "system.indexes")]:
                models.mongo.db.drop_collection(collection)

    def test001_view(self):
        with app.test_request_context():
            result = views.view()
            self.assertEquals(result,
                              "<h1>Oh boy!</h1>You got 0 tasks on yo base!")

    def test002_tasks(self):
        with app.test_request_context():
            models.new_object("test")
            result = views.tasks()
            self.assertEquals(result,
                              "<table><tr><th>Number</th><th>String</th></tr><tr><td>0</td><td>test</td></tr></table>")

class test_api(TestCase):

    def test001_show(self):
        with app.test_request_context():
            result = api.show()
            self.assertIsInstance(result, Response)
            self.assertEquals(result.status_code, 200)
            self.assertEquals(loads(result.data), {"result": []})

    def test002_insert(self):
        old_func = api.new_object
        api.new_object = lambda x: True
        with app.test_request_context():
            result = api.insert("test")
            self.assertIsInstance(result, Response)
            self.assertEquals(result.status_code, 200)
            self.assertEquals(loads(result.data), {"msg": "ok"})
        api.new_object = old_func

Bon, ça a beau faire une couverture à 100%, c’est tout de même assez dégueulasse dans l’esprit.
Les gars de Flask sont sympas, on a des méthodes exprès (Flask.test_client, Flask.test_request_context) pour créer des contextes et faire nos tests… C’est un peu comme si un gouvernement taxait votre travail à 50% et ensuite vous fasse des remises dans certaines conditions [1] : c’est toujours ça de gagné, mais ça reste une vaste fumisterie à la base.
Pour tout dire, à voir à quel point ce qu’on teste est basique, ça fait un peu pitié d’avoir des tests si encombrés et peu clairs. Et puis quand ça sera moins basique, je vous jure que c’est franchement immonde à configurer.

Où on en est après ça ?

  • Un code bien à plat, assez moche, rempli de dépendances (douteuses) à Flask un peu partout qui figent le comportement et freinent fortement sa bonification (nettoyage de code et factorisation)
  • des imports hasardeux qui révèlent que l’init du package ne consiste pas seulement en l’instanciation de l’app Flask (ce qui déjà en soit est moche) mais aussi à l’interprétation des fichiers views.py et models.py pour faire le mapping entre URLs et fonctions;
  • des tests bien lourds qui donnent pas envie (et dont le maintien sera donc vraisemblablement abandonné sous peu, soyons honnêtes).
    Soulignons en passant, dans le test test_api.test002_insert, le petit mock sur la fonction api.new_object (ce n’est pas ça qu’on veut tester ici, alors on le remplace par un truc qui passe quoi qu’il arrive). Du fait que le module api que l’on manipule ici est le même que dans tous les autres tests, on est obligé de faire ces très inélégantes étapes de 1) sauvegarder l’ancienne fonction, 2) mettre le mock, puis 3) remettre l’ancienne fonction une fois le test terminé.
    D’une part, c’est laid; d’autre part, c’est clairement une pirouette qu’on a tendance à oublier de faire, et après on ne comprend pas pourquoi nos tests ont des comportement erratiques alors que la moitié des fonctions se retrouvent remplacées par des mocks…

Bref globalement on est très loin d’avoir fait un quelconque programme. Je considère qu’à ce niveau nous disposons au mieux d’un script qui fournit dans un ordre donné des informations à Flask. L’intelligence que l’on apporte doit être entièrement consommée par le framework et ne peut être utilisée simplement par personne d’autre. Sans Flask, ce code ne vaut donc rien, puisqu’on ne peut rien en tirer pour un autre usage.

Pourtant, en tant que développeur, il est tout à fait concevable qu’on se dise un jour « tient, cette génération HTML est intéressante, je pourrais l’utiliser ailleurs » ou encore « ce travail au-dessus de mongodb pourrait me servir sur un projet similaire ». Ou alors tout d’un coup on veut ajouter un thread à notre appli pour faire de l’administration ou quoi que ce soit d’autre, et on veut – comme par hasard – exploiter du code qu’on a déjà fait et qui est là, sous notre main…

Et bien on ne peut pas – pas encore.

Second code

Pour pallier en parti à ces défauts, j’ai tendance à passer sur un code de ce genre-là :

run.py
pack/
 |--- __init__.py
 |--- application.py
 |--- models.py
 |--- views.py
 |--- api.py
test/
 |--- tests.py
#!/usr/bin/env python2.7

from pack import app
if __name__ == "__main__":    
    app.run()
from application import Application

app = Application()
import pymongo
from flask import Flask
from pack.api import Api
from pack.views import Views
from pack.models import Models

class Application(Flask):

      def __init__(self):
            super(Application, self).__init__(__name__)
            self.mongo = pymongo.Connection()["my_database"]
            self.models = Models(self.mongo)
            self.api = Api(self.mongo, self.models)
            self.views = Views(self.mongo)
            self.define_routes()

      def define_routes(self):
            api_routes = {
                  "/api/info/": self.api.show,
                  "/api/insert/<string:ma_string>": self.api.insert
                  }
            views_routes = {
                  "/": self.views.view,
                  "/tasks/": self.views.tasks
                  }
            for route, method in api_routes.items() + views_routes.items():
                  self.add_url_rule(route, view_func=method)
class Models(object):

    def __init__(self, database):
        self.mongo = database

    def new_object(self, string):
        number = self.mongo.my_collection.count()
        self.mongo.my_collection.insert({"string": string, "num": number})
class Views(object):

    def __init__(self, database):
        self.mongo = database
        
    def view(self):
        num = self.mongo.my_collection.count()
        return "<h1>Oh boy!</h1>You got %s tasks on yo base!" % num, 200

    def tasks(self):
        tasks = self.mongo.my_collection.find({}, {"_id": False})
        string = "<table><tr><th>Number</th><th>String</th>"
        for task in tasks:
            string += "<tr><td>%s</td><td>%s</td></tr>" % (task["num"],
                                                           task["string"])
        return string + "</table>", 200
from bson.json_util import dumps

class Api(object):

    def __init__(self, database, models):
        self.mongo = database
        self.models = models

    def show(self):
        result = {"result": list(self.mongo.my_collection.find({}, {"_id": False}))}
        return dumps(result), 200

    def insert(self, ma_string):
        self.models.new_object(ma_string)
        return dumps({"msg": "ok"}), 200

 J’ai la faiblesse de penser que ce code est infiniment plus propre, plus facile à tester, plus facile à factoriser, plus facile à porter vers d’autres programmes que le précédent.

Certes il a des points négatifs. Il est plus long (75 lignes au lieu de 51, ce qui fait tout de même près de 50% de plus), et s’il peut paraître plus clair pour quelqu’un avec un bagage Python raisonnable, je comprends qu’il puisse paraître relativement plus obscure si on débarque tout juste dans le langage et qu’on n’a pas de notion de programmation orientée objets.

Je m’autorise de plus certains raccourcis. J’estime par exemple que jsonify est parfaitement dispensable (complexité inutile, et en plus je ne sais pas pourquoi mais je galère à le taper), tout comme le wrapper sur pymongo.

À côté de ça cependant, on découvre que chaque classe a sa propre indépendance, et est surtout totalement découplée de Flask (mise à part la classe Application évidemment, qui sert justement de point de référence contrôlé vers Flask).
La construction de l’application est synthétisée dans la classe Application (et non plus dispersé un peu partout dans le code via des décorateurs moches qui en plus entravent violemment l’accès aux fonctions du fait du contexte qui doit être créé), ce qui rend immédiatement lisible l’architecture logiciel et décharge les autres classes de surcouches sans rapport avec leur boulot (ce qui nous rapproche du single responsibility principle).
Il devient extrêmement facile de réutiliser une classe dans ce cadre, et les tests deviennent plus propres, puisqu’on peut enfin tester ce qu’on veut sans se manger une erreur de contexte dans la trogne.

import pymongo
from bson.json_util import loads
from unittest import TestCase

from pack.views import Views
from pack.models import Models
from pack.api import Api

DB = pymongo.Connection()["test"]

class test_Models(TestCase):

    def setUp(self):
        self.models = Models(DB)

    def tearDown(self):
        for collection in [item for item in DB.collection_names()
            if not (item == "system.indexes")]:
            DB.drop_collection(collection)

    def test001_new_object(self):
        self.models.new_object("test")
        result = list(DB.my_collection.find({}, {"_id": False}))
        self.assertEquals(len(result), 1)
        self.assertEquals(result[0], {"string": "test", "num": 0})

class test_Api(TestCase):

    def setUp(self):
        self.api = Api(DB, Models(DB))

    def tearDown(self):
        for collection in [item for item in DB.collection_names()
            if not (item == "system.indexes")]:
            DB.drop_collection(collection)

    def test001_show(self):
        result = self.api.show()
        self.assertIsInstance(result, tuple)
        self.assertEquals(result[1], 200)
        self.assertEquals(loads(result[0]), {"result": []})

    def test002_insert(self):
        self.api.models.new_object= lambda x: True
        result = self.api.insert("test")
        self.assertIsInstance(result, tuple)
        self.assertEquals(result[1], 200)
        self.assertEquals(loads(result[0]), {"msg": "ok"})

class test_Views(TestCase):

    def setUp(self):
        self.views = Views(DB)
        self.models = Models(DB)

    def tearDown(self):
        for collection in [item for item in DB.collection_names()
        if not (item == "system.indexes")]:
                DB.drop_collection(collection)

    def test001_view(self):
        result = self.views.view()
        self.assertEquals(result,
                          "<h1>Oh boy!</h1>You got 0 tasks on yo base!")

    def test002_tasks(self):
        self.models.new_object("test")
        result = self.views.tasks()
        self.assertEquals(result,
                          "<table><tr><th>Number</th><th>String</th></tr><tr><td>0</td><td>test</td></tr></table>")

 Effectivement, c’est de nouveau un peu plus long qu’à l’origine (+7 lignes[2]) car il faut prendre le temps d’instancier nos classes dans les setUp. Sur une batterie de tests plus trapue cependant, on va vraisemblablement gagner de la place.
Surtout cette fois-ci on peut réellement se concentrer sur ce qu’on doit tester, pas sur de l’auxiliaire.

Tiens, en passant, dans test_api.test002_insert, on n’a même plus besoin de sauvegarder l’ancienne fonction que l’on mock, puisque l’api sera réinstanciée au test suivant !

Petit bonus : on gagne du temps ! Les tests précédents prenaient en moyenne 120,8ms, les nouveaux ne prennent que 106,2ms. Imaginez lorsque vous aurez plusieurs centaines de tests !

Great success

Ce n’est pas fini

Ce n’est jamais fini.

On pourrait commencer par remonter l’instanciantion d’Application dans le run.py. De cette façon on obtiendrait réellement une bibliothèque d’un côté (comme je l’entends : un ensemble de classes qui forment un tout indépendant et qu’il faut instancier pour les utiliser) et un script de lancement de l’application de l’autre (le run.py qui instancie notre application et la lance).

Il faut évidemment dégager le code HTML vers des templates extérieurs. C’est réellement trivial avec Jinja2, mais comme je suis une feignasse, je vous laisse le boulot.

Vous avez bien sûr remarqué que pas mal de code pouvait être factorisé (les instanciations des classes Views / Api / Models assez similaires, les tearDown dans les tests). Plus qu’à faire jouer l’héritage !
Il faudrait aussi faire une vraie interface vers la base de données, c’est un peu moyen de balader mongo un peu partout avec des accès directement depuis Api ou Views.

En plus des routes classiques (@route), le code comprend probablement des décorateurs pour la gestion d’erreur (@errorhandler) et pour préfixer/postfixer les requêtes (@before_request, @after_request). Vous pouvez facilement vous en passer en exploitant les champs before_request_funcs (ou after) et error_handler_spec de votre instance de Flask (au hasard, à l’initialisation d’Application).
Attention cependant si vous utilisez la classe LoginManager de flask.ext.login, cette dernière va manipuler les champs before_request_funcs (et after); il faut donc veiller à ne pas écraser ces modifications.

On pourrait aussi extraire réellement le code métier de Views et Api. Dans l’idée, ces deux classes souvent font globalement la même chose (à part qu’elles présentent les informations dans des formats différents), ce qui fait qu’il n’est pas rare de voir Views exploiter des méthodes d’Api (dont les retours sont plus « bruts »). Il vaut mieux se constituer une vraie classe métier fournissant des informations indifféremment à Views et Api, qui elles se concentrent uniquement sur leur rôle d’interface (réception et traitement des requêtes, génération de réponses).

Si on utilise request (flask.request), il faudrait le neutraliser dans une classe à part. C’est vraisemblablement une étape peu évidente, car request est certainement la globale de Flask qui colle le plus au code tellement on a tendance à l’utiliser à outrance (il faut avouer qu’elle est bien pratique dès qu’on fait du POST).

Autres trucs un peu moisis dans Flask

J’avoue avoir été étonné de l’engouement autour de ce framework. Il ne fait pourtant pas grand chose de plus (à par nous emmerder) que beaucoup d’autres…

J’ai quand même pris le temps de regarder un peu tout de même pour être sûr de ne pas passer à côté de quelque chose… Petite sélection :

  • L’existence de la méthode Flask.make_reponse est assez dingue en soit. Elle illustre parfaitement le côté « boah, les views et api peuvent bien retourner n’importe quoi, en fait on s’en fout ». Jusqu’à maintenant tu pouvais renvoyer une string que Flask se chargeait de passer en werkzeug.Response, mais bon, si tu veux toi-même renvoyer une Response… La souplesse c’est bien, mais là c’est une invitation à faire du code malsain.
    Au moins avec ça on est assuré que passer de jsonify à bson.json_util.dumps ne change rien (et que jsonify est réellement inutile).
  • La palme de la globale la plus vicieuse revient sans conteste à request (from flask import request). Pour peu que vous travailliez avec une boîte noire en face, qui vous envoie du JSON pas forcément de la bonne façon (headers manquant j’imagine), l’accès aux données dans request devient un peu folklorique. Ça m’a au moins permis d’apprendre l’existence des MultiDict (mais là on parle plus d’une sournoiserie de werkzeug).
    Au final, on ne peut vraiment faire confiance qu’à request.get_data() – qui est deprecated et est vouée à disparaître…

 Conclusion of ze dead

Je vais être un peu tendre : Flask, on peut l’utiliser, ça peut servir. Le problème c’est qu’il faudrait l’utiliser de façon totalement antinaturelle et opposée à tout ce qu’on peut trouver dans ses tutos, si on voulait l’exploiter pour autre chose qu’un projet étudiant.

Vous êtes libre de faire comme vous le sentez. Mais gardez toujoursune pensée pour le pauvre type qui devra reprendre votre code derrière vous.

 

1. Toute ressemblance avec des cas réels et contemporains ne serait évidemment que pur et fortuit hasard.

2. En sachant que la classe Application n’est pas testée ici. En tant que surcouche à Flask, il faudrait vérifier au moins le type de ses attributs et l’existence des routes.

Laisser un commentaire

Votre adresse e-mail ne sera pas publiée. Les champs obligatoires sont indiqués avec *

Ce site utilise Akismet pour réduire les indésirables. En savoir plus sur comment les données de vos commentaires sont utilisées.