Produits Entr'ouvert » Passerelle

Benjamin Dauvergne a écrit :

De cet id on pourrait générer la requête reverse suivante en cas d'absence de cache :

J'aime bien cette idée (en évitant cependant l'usage d'espaces dans le "id" mais c'est juste moi et je me soigne)

0002 : 128 caractères ça parait assez court, non ? Je pense qu'il faut stocker la donnée "q" le plus précisément possible.

Je pense qu'il y a une erreur avec le « return results » en fin de :

         # Use search with label as q and lat/lon as geographic hint
        if q and lat and lon:
            results = self.addresses(request, q=q, lat=lat, lon=lon, citycode=citycode)['data']
            for result in results:  # match by id if possible
                if result['ban_id'] == ban_id:
                    return {'data': [result]}
            return results

ça serait plutôt « return {'err': _('Address ID not found')} » non ?

Thomas Noël a écrit :

0002 : 128 caractères ça parait assez court, non ? Je pense qu'il faut stocker la donnée "q" le plus précisément possible.

L'unicité est acquise par l'id BAN de toute façon, sans compter lat/lon; en fait on pourrait ne pas utiliser toute la chaîne pour ça, j'ai hésité à faire une version où je tronquerai l'id avant de créer l'objet AddressCache, mais j'ai eu peur qu'on m'accuse de complexifier. Mais tu as raison 128 caractères c'est à la fois trop pour l'unicité mais trop peu pour garantir que tout rentrera toujours, et on risque donc d'avoir des erreurs de donnée trop longue, je vais revenir à 30 et simplement tronquer.

Je pense qu'il y a une erreur avec le « return results » en fin de :
[...]

ça serait plutôt « return {'err': _('Address ID not found')} » non ?

Ça demande un commentaire dans le code: l'idée c'est que dans l'éventualité rare, mais pas impossible, que l'id change, on parie sur le fait que le premier résultat (search les rangeant par distance à la position géographique et à la chaîne de recherche, je ne sais pas avec quelle pondération, surtout sur l'implémentation de Toulouse, qui n'est pas addok je pense) sera le bon (w.c.s. prenant le premier résultat de la liste comme valeur de la source de donnée pour cette id); dans l'idée qu'on arriverait à retomber sur nos pieds dans ce cas limite.

Je suis revenu à 30 caractères, en tronquant chaque fois qu'il faut, et j'ai ajouté un commentaire pour expliquer pourquoi on ne renvoie pas une erreur.

Benjamin Dauvergne a écrit :

Thomas Noël a écrit :

0002 : 128 caractères ça parait assez court, non ? Je pense qu'il faut stocker la donnée "q" le plus précisément possible.

L'unicité est acquise par l'id BAN de toute façon, sans compter lat/lon; en fait on pourrait ne pas utiliser toute la chaîne pour ça, j'ai hésité à faire une version où je tronquerai l'id avant de créer l'objet AddressCache, mais j'ai eu peur qu'on m'accuse de complexifier. Mais tu as raison 128 caractères c'est à la fois trop pour l'unicité mais trop peu pour garantir que tout rentrera toujours, et on risque donc d'avoir des erreurs de donnée trop longue, je vais revenir à 30 et simplement tronquer.

Bon je me suis perdu tout seul, j'avais mal lu et compris que le cet id à 128 caractère contenait le q= et non. Mais donc en relisant mieux, le q= est contenu dans l'id retourné par le search et ça, ça ne me va pas vraiment. Ca fait des id vraiment très moches et je me demande comment w.c.s. va interpréter ça (ok normalement rien ne devrait planter, mais quand même, des espaces, des accents, etc.). Je n'avais pas perçu que cet idée de se rebaser sur "search" amenait ce problème (plutôt que d'utiliser reverse comme ton idée initiale, qui n'avait besoin que de lat et lon).

Du coup je ne sais plus trop quoi penser... :/ Pourquoi finalement tu n'étais pas parti sur l'usage de reverse ?

ça serait plutôt « return {'err': _('Address ID not found')} » non ?

Ça demande un commentaire dans le code: l'idée c'est que dans l'éventualité rare, mais pas impossible, que l'id change, on parie sur le fait que le premier résultat (search les rangeant par distance à la position géographique et à la chaîne de recherche, je ne sais pas avec quelle pondération, surtout sur l'implémentation de Toulouse, qui n'est pas addok je pense) sera le bon (w.c.s. prenant le premier résultat de la liste comme valeur de la source de donnée pour cette id); dans l'idée qu'on arriverait à retomber sur nos pieds dans ce cas limite.

Mouais mais non, retourner un résultat dont l'id n'est pas le même que celui demandé, ça va être un enfer à déboguer le jour où. Ca me semble plus raisonnable de considérer que l'id n'existe plus dans la BAN, et oui ça va planter l'édition, et c'est normal : l'adresse n'existe plus.

Thomas Noël a écrit :

Benjamin Dauvergne a écrit :

Thomas Noël a écrit :

0002 : 128 caractères ça parait assez court, non ? Je pense qu'il faut stocker la donnée "q" le plus précisément possible.

L'unicité est acquise par l'id BAN de toute façon, sans compter lat/lon; en fait on pourrait ne pas utiliser toute la chaîne pour ça, j'ai hésité à faire une version où je tronquerai l'id avant de créer l'objet AddressCache, mais j'ai eu peur qu'on m'accuse de complexifier. Mais tu as raison 128 caractères c'est à la fois trop pour l'unicité mais trop peu pour garantir que tout rentrera toujours, et on risque donc d'avoir des erreurs de donnée trop longue, je vais revenir à 30 et simplement tronquer.

Bon je me suis perdu tout seul, j'avais mal lu et compris que le cet id à 128 caractère contenait le q= et non. Mais donc en relisant mieux, le q= est contenu dans l'id retourné par le search et ça, ça ne me va pas vraiment. Ca fait des id vraiment très moches et je me demande comment w.c.s. va interpréter ça (ok normalement rien ne devrait planter, mais quand même, des espaces, des accents, etc.). Je n'avais pas perçu que cet idée de se rebaser sur "search" amenait ce problème (plutôt que d'utiliser reverse comme ton idée initiale, qui n'avait besoin que de lat et lon).

Du coup je ne sais plus trop quoi penser... :/ Pourquoi finalement tu n'étais pas parti sur l'usage de reverse ?

Le fait que reverse ne se base que sur la distance à lat/lon alors que search va utiliser le nom et lat/lon en même temps,

Mouais mais non, retourner un résultat dont l'id n'est pas le même que celui demandé, ça va être un enfer à déboguer le jour où. Ca me semble plus raisonnable de considérer que l'id n'existe plus dans la BAN, et oui ça va planter l'édition, et c'est normal : l'adresse n'existe plus.

Ok, je reviens au patch du départ alors.

Benjamin Dauvergne a écrit :

[...]

Poussé avec la modif demandée aux tests.