Development #51327
misc: interdire l'appel à data_class() sur un {form,card]def léger
0%
Description
Sait on jamais.
Fichiers
Historique
Mis à jour par Benjamin Dauvergne il y a environ 3 ans
- Fichier 0001-formdef-forbid-use-of-data_class-on-lightweight-form.patch 0001-formdef-forbid-use-of-data_class-on-lightweight-form.patch ajouté
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
Mis à jour par Benjamin Dauvergne il y a environ 3 ans
Le fait que les tests passent constituera déjà une vérification que la situation n'existe pas de manière évidente actuellement.
Mis à jour par Benjamin Dauvergne il y a environ 3 ans
- Fichier 0003-tests-allow-exceptions-to-backtrace-into-tests.patch 0003-tests-allow-exceptions-to-backtrace-into-tests.patch ajouté
- Fichier 0001-formdef-forbid-use-of-data_class-on-lightweight-form.patch 0001-formdef-forbid-use-of-data_class-on-lightweight-form.patch ajouté
- Fichier 0002-formdef-prevent-hiding-of-EOFError-on-new-style-pick.patch 0002-formdef-prevent-hiding-of-EOFError-on-new-style-pick.patch ajouté
1. ne sert pas à grand chose pour l'instant, mais ça garantit qu'on ne fait pas n'importe quoi avec data_class()
2. parce que je soupçonne qu'on est pas aidé par NFS et qu'on est un peu trop permissif avec les EOFError je ne les laisse plus passer dans storage_load() sauf si on est bien sûr qu'on est sur un pickle vieux format (où o.field devrait forcément être une liste)
3. parce que je ne voyais pas les exceptions dans 1.
Mis à jour par Frédéric Péters il y a environ 3 ans
C'est rouge mais je commente quand même :
- 0001 : en plus de lever l'erreur il y a modification pour créer des lightweight là où ça n'était pas le cas, il ne faudrait pas ça dans de patch.
- 0003 : pas fan du tout de changer de manière si totale le comportement, je préfère vraiment garder les tests se comporter comme l'application, et qu'un problème y soit annoncé comme une erreur 500.
Mis à jour par Frédéric Péters il y a environ 3 ans
- Statut changé de Solution proposée à En cours
- Patch proposed changé de Oui à Non
Mis à jour par Benjamin Dauvergne il y a environ 3 ans
Oui après d'autres tests sur le SaaS HDS je me fourvoie certainement, il n'y a pas de NFS là bas et l'écriture atomique marche parfaitement (j'ai lancé plein de process qui font open(.tmp.<random>).write(' ' * 10000).flush().rename('x'), je n'ai jamais réussi à trouver autre chose que ma chaîne de 10000 caractères dans 'x' avec d'autres process concurrents qui lisent).
Je vais juste continuer sur l'aspect surveiller un peu plus quand on utilise dataclass() sur un FormDef léger.
Mis à jour par Benjamin Dauvergne il y a environ 3 ans
Frédéric Péters a écrit :
- 0003 : pas fan du tout de changer de manière si totale le comportement, je préfère vraiment garder les tests se comporter comme l'application, et qu'un problème y soit annoncé comme une erreur 500.
Oui je comprends, je ne pensais pas le pousser à la fin, dans l'absolu ça pourrait être sympa d'avoir quelque chose pour chaîne l'exception initiale à celle émise par webtest pour la 500.
Mis à jour par Benjamin Dauvergne il y a environ 3 ans
- Fichier 0003-tests-chain-webtest-500-exceptions-to-the-internal-e.patch 0003-tests-chain-webtest-500-exceptions-to-the-internal-e.patch ajouté
- Fichier 0001-formdef-forbid-use-of-data_class-on-lightweight-form.patch 0001-formdef-forbid-use-of-data_class-on-lightweight-form.patch ajouté
- Fichier 0002-formdef-prevent-hiding-of-EOFError-on-new-style-pick.patch 0002-formdef-prevent-hiding-of-EOFError-on-new-style-pick.patch ajouté
- Statut changé de En cours à Solution proposée
- Patch proposed changé de Non à Oui
Je veux bien faire un ticket à part pour 0003 si c'est jugé un peu cavalier dans ce ticket. Je précise que ce n'est pas le même code que le 0003 précédent, c'est plus élégant, ça lève les mêmes erreurs webtest qu'avant, simplement ça lie l'exception à l'exception interne permettant d'avoir une trace complète du problème.
Mis à jour par Benjamin Dauvergne il y a environ 3 ans
Frédéric Péters a écrit :
(je ne souhaite vraiment pas ce 0003).
Ok, est-ce que tu peux relire le reste ?
Mis à jour par Frédéric Péters il y a environ 3 ans
- Statut changé de Solution proposée à Fermé
Vu en écrivant #52127, on était déjà protégé ici,
if getattr(formdef, 'fields', None) is Ellipsis:
# don't touch tables for lightweight objects
return []