Produits Entr'ouvert » Passerelle

Je prends.

• 0001:
  • je conseille d'utilise django.utils.http.urlencode qui fonctionne mieux en général et surtout avec de l'unicode (Django décore les paramètres)
  • il me semble que si tu as une erreur de type requests.exceptions.RequestException tu n'as rien pour en faire une APIError propre (en fait c'est vrai pour tout le module, faudrait gérer ça dans la méthode request(). C'est vrai pour tout passerelle je dirai même, je ne vois que le module iParapheur qui fasse un import depuis requests.exceptions,
  • je n'écraserai pas response avec sa valeur JSON, juste au cas où que ce soit important dans une trace, plutôt faire content = response.json()
  • je mettrai un commentaire pour dire que la valeur doit toujours être un dico avec une seule clé ex.: {"departements": [...]} comme je le vois dans les tests, j'irai même jusqu'à faire un test explicite juste avant:

    if not isinstance(content, dict) or len(content) != 1 or not istinstance(content.values()[0], list):
        raise APIError('response is not a dictionnary with only one key and whose value is a list', data={'json_content': content})
    

  • idem pour le contenu de items:

    if not all(isinstance(item, dict) and item.get('code') for item in items):
        raise APIError('items must be dictionnaries with a "code" key', data={'json_content': content})
    

  • je pense que tu peux te permettre d'introduire simplify dans passerelle.utils
  • je pense que ce serait plus claire ça:

    def condition(item):
        if ignore and item['id'] in ignore_ids:
            return False 
        if q and q not in simplify(item['text']):
            return False
        return True
    items = filter(condition, items)
    

    
    que ça
    

            items = [item for item in items
                        if (not q or (q in simplify(item['text'])))
                            and (not ignore or (item['id'] not in ignore_ids))]
    

• À voir pour plus tard mais c'est typiquement le genre de endpoint où du cache d'environ quelques dizaines de secondes ne pourrait pas faire de mal (mais ça peut aussi ne servir à rien du tout, donc à garder sous le coude)
  0002: ben même remarques que plus haut, gérer les RequestException, à voir si il vaut mieux pas le faire au niveau de passerelle.utils.Request, voir à sous classer APIError pour un APIRequestError quitte à pouvoir les catcher si on a quelque chose de mieux à dire.

je pense que tu peux te permettre d'introduire simplify dans passerelle.utils

Il y a déjà un slugify dans django.utils.text, le code peut ici faire quelque chose de subtilement différent mais si cette subtilité est pertinente pour l'ensemble de passerelle il faudra l'expliciter, qu'on sache quand utiliser l'une ou l'autre.

Frédéric Péters a écrit :

je pense que tu peux te permettre d'introduire simplify dans passerelle.utils

Il y a déjà un slugify dans django.utils.text, le code peut ici faire quelque chose de subtilement différent mais si cette subtilité est pertinente pour l'ensemble de passerelle il faudra l'expliciter, qu'on sache quand utiliser l'une ou l'autre.

Ici c'est pour aider la recherche de noms de lieux, en fait, j'avais envie de reprendre l'idée qui est dans wcs, de comparer des chaines "traduites" en ascii, avec une tentative d'intelligence sur les espace, trait d'union, apostrophe, etc.

Thomas Noël a écrit :

• il me semble que si tu as une erreur de type requests.exceptions.RequestException tu n'as rien pour en faire une APIError propre (en fait c'est vrai pour tout le module, faudrait gérer ça dans la méthode request(). C'est vrai pour tout passerelle je dirai même, je ne vois que le module iParapheur qui fasse un import depuis requests.exceptions,

C'est le monde tordu des proxies... que faire quand le proxy n'arrive pas à faire son travail, quelle erreur renvoyer ? Pour l'instant ces exceptions qu'on n'attrape pas provoquent une 500, mais bien en JSON avec le err=1 (grâce au décorateur endpoint et jsonresponse). Que faire d'autre, quel autre code d'erreur utiliser, sachant que 200 est exclu car il veut dire "j'ai réussi mon boulot mais de l'autre côté ça déconne" ? Ici Passerelle a bien échoué dans sa mission, et si ça se trouve le crash est vraiment dans sa propre config DNS ou firewall, et donc la 500 n'est pas si anormale... Pour l'instant on vit avec ça, je n'ai pas d'autre idée. (mais cf en bas du ticket).

Je trouve juste fatiguant de recevoir un traceback pour un Timeout ou une erreur SSL ou une erreur DNS, pour moi c'est pas la faute à passerelle si le firewall est pas ouvert ou le DNS déconne, il a fait son job, c'est le monde qui est contre lui ;)

0002: ben même remarques que plus haut, gérer les RequestException, à voir si il vaut mieux pas le faire au niveau de passerelle.utils.Request, voir à sous classer APIError pour un APIRequestError quitte à pouvoir les catcher si on a quelque chose de mieux à dire.

Je pense qu'il faut faire un autre ticket sur "comment réagir en cas de pépin lors de tel ou tel request", via un ou des paramètres à envoyer au self.requests, qui déclencherait une exception proprement rattrapée par le jsonresponse, avec du log plus adéquat, genre... Mais je n'ai pas envie de faire ça ici maintenant.

Ok je veux bien que tu ouvres le tiquet puisque tu sembles avoir un avis plus tranché que le mien sur ce qui a le droit d'être une erreur propre ou pas :)

Ici c'est pour aider la recherche de noms de lieux, en fait, j'avais envie de reprendre l'idée qui est dans wcs, de comparer des chaines "traduites" en ascii, avec une tentative d'intelligence sur les espace, trait d'union, apostrophe, etc.

Je préférerais vraiment garder ça dans le connecteur pour le moment.

Benjamin Dauvergne a écrit :

Ok je veux bien que tu ouvres le tiquet puisque tu sembles avoir un avis plus tranché que le mien sur ce qui a le droit d'être une erreur propre ou pas :)

Hop #21653, Et à relire, sans doute qu'on peut faire quelque chose de simple pour éviter le logger.exception dans jsonresponse.