Produits Entr'ouvert » w.c.s.

Pourtant dans wcs/wf/profile.py :

                if field and field_value and field.convert_value_from_anything:
                    field_value = field.convert_value_from_anything(field_value)
                new_user_data[field.id] = field_value

et convert_value_from_anything('') renvoie bien un None pour un DateField.

Je dois rater un truc ...?

and field_value, donc on ne passe pas dedans.

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

Après avoir vérifié que convert_value_from_anything provoque bien des ValueError si vide ou None est reçu (pour DateField et FileField, seuls champs avec cette méthode) [...]

Je me perds là; pour moi convert_value_from_anything a le comportement de retourner None en cas de valeur vide pour le DateField (ce qui me semble le comportement souhaité ici, vu qu'on n'entoure pas tous les appels à convert_value_from_anything de try/except ValueError); mais FileField n'a pas ce comportement (et lève un ValueError mais dans le test qu'on a c'est dans le contexte d'un champ backoffice et là il y a un try/except).

Ah oui, je m'étais perdu aussi lors de mes lectures/patches sur le sujet. Effectivement le comportement est différent entre les deux convert de DateField et FileField.

Sur l'idée qu'on ne va pas autoriser tout de suite les champs fichier dans le profil, ack, mais je pense qu'on devrait dégager sans doute dégager le ValueError du convert_value_from_anything de FileField. (et sans doute y avoir un explicite "if not value: return None", puis peut-être le notify_of_exception(...) qu'on a dans wf/backoffice_fields.py.

MMMhhhh... en fait, y'a deux cas, soit la value reçu est '' et alors c'est un soucis (mauvais type de variable, quelqu'un a du essayer de taper [bidule] ou {{bidule_raw}} dans l'affectation), soit c'est None et alors c'est "normal" (pas de fichier à stocker).

Je me dis que ça devrait être pareil pour les DateField (prendre None sans râler).

Aussi, on a rencontré l'erreur lors d'un provisionning SAML sur la prod, et j'ai donc appliqué ça en hot fix.

Ok.

(Et au cas où j'ai encore les tests qui tournent en local, j'annulerai le ack s'ils échouent)

Ouaip j'ai pas joué les tests sur la partie SAML, un peu trop chiant, j'avoue.

MMMhhhh... en fait, y'a deux cas, soit la value reçu est '' et alors c'est un soucis (mauvais type de variable, quelqu'un a du essayer de taper [bidule] ou {{bidule_raw}} dans l'affectation), soit c'est None et alors c'est "normal" (pas de fichier à stocker).

Sauf que pour un champ date vide Authentic envoie un message de provisionning avec une chaine vide, pas null. (c'est ça qu'on a eu comme trace hier)

Je me dis que ça devrait être pareil pour les DateField (prendre None sans râler).

C'est le cas avec ton patch mais on attrapait déjà le ValueError silencieusement, pas de changement ici.

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

MMMhhhh... en fait, y'a deux cas, soit la value reçu est '' et alors c'est un soucis (mauvais type de variable, quelqu'un a du essayer de taper [bidule] ou {{bidule_raw}} dans l'affectation), soit c'est None et alors c'est "normal" (pas de fichier à stocker).

Sauf que pour un champ date vide Authentic envoie un message de provisionning avec une chaine vide, pas null. (c'est ça qu'on a eu comme trace hier)

Que je pige : tu verrais donc un retour de None "normal" si value est '', pour DateField.convert_value_from_anything ? Pourquoi pas, hein...

Je me dis que ça devrait être pareil pour les DateField (prendre None sans râler).

C'est le cas avec ton patch mais on attrapait déjà le ValueError silencieusement, pas de changement ici.

Oui ça change rien, ça «nettoie» juste le code.

En fait je vois pas trop ce qui pose problème dans mon patch, y'a quelque chose que je dois rater.

Que je pige : tu verrais donc un retour de None "normal" si value est '', pour DateField.convert_value_from_anything ? Pourquoi pas, hein...

Oh là là non, je ne veux rien du tout, et le patch précédent m'allait, et celui-ci aussi, je notais juste qu'ils ne me semblaient en pratique rien changer.

_~~

Mais à discuter, en "non-pratique", tu introduis ici une notion de retour "normal" mais il se définit soit en opposition au traitement de la chaine vide pour les dates (qui retourne None, mais un None "anormal"). Ou en opposition au traitement d'une chaine vide dans FileField (qui ne retourne pas None mais lève un ValueError).

Et donc, à prendre ce chemin, traiter de manière "normale" le None, appelle pour moi aussi le besoin d'unifier le traitement de la chaine vide "anormale", que ça soit en retournant tout le temps None dans FileField, ou le levant aussi ValueError dans DateField.

Bref, s'il doit y avoir nettoyage, pour moi, ça doit inclure le traitement uniforme des valeurs "anormales", pas juste le traitement du None comme valeur "normale".

En cas de ValueError, je déclenche des publisher.notify_of_exception dans les différents contextes hobo, saml, profil et champs backoffice. Il est possible que cela provoque des alertes mail qu'on n'a pas actuellement sur les contextes hobo ou saml, parce qu'on ne passe pas par ce code actuellement (if value and ...). Je propose de poser sur la en recette, et si on reçoit trop d'alertes retirer les notify ou, si possible, corriger le soucis à la source (a2, hobo, ...).

Sur la recette rien reçu mais de la prod on vient de recevoir une notif lors de provisionning, parce que dans la notif hobo birthdate était une chaine vide (compte strasbourg avec l'uuid be74b487c458491d9fe50645100b3a5f).