Produits Entr'ouvert » Authentic 2

Dans le même genre, https://github.com/raphaelm/django-jsonfallback/, mais c'est plutôt dans une optique de support post Django 1.11 notamment pour mysql et sqlite.

Une première relecture, avant d'appliquer le patch et de le tester en vrai :

Dans authentic2/apps.py :

Je pense qu'on peut laisser expected_type en minuscules
et donc retirer le upper() avant l'assignation à current_type.

J'ai l'impression que django.db.models.fields.Field.get_attname_column ne remonte par d'erreur si la colonne n'est pas trouvée, à la place il renvoie le nom d'attribut.
Je serais donc vigilant lorsqu'on fait un
cursor.fetchone()[0].upper()

Dans authentic2/compat.py :

Dans le constructeur de authentic2.compat.JSONField :

        except ImportError:
            raise
            pass

la méthode contribute_to_class doit supporter un paramètre private_only (et, respecter l'interface, son équivalent déprécié virtual_only).

D'ailleurs cette méthode, dans le code django de contrib postgres se termine par un bout de code qui me paraît nécessaire aussi ici :

        if self.column:                                                                             
            # Don't override classmethods with the descriptor. This means that                      
            # if you have a classmethod and a field with the same name, then                        
            # such fields can't be deferred (we don't have a check for this).                       
            if not getattr(cls, self.attname, None):                                                
                setattr(cls, self.attname, DeferredAttribute(self.attname, cls)) 

Dans la méthode compat.JSONField.deconstruct :
Je ne comprends ce bout de code :

        if inspect.getframeinfo(previous_frame)[2] in ('serialize', 'deep_deconstruct'):
            d = d[1:]

Il faudra aussi m'expliquer d'où vienne jsonfield.fields.configure_database_connection et jsonfield.fields.connection_created.
Je ne trouve pas le code, et donc je ne peux pas comprendre ce que tu cherches à faire avec ces quelques lignes à la fin de compat.py.

Dans tests/settings.py :

+        'NAME': 'postgres',

Wut ?!

Paul Marillonnet a écrit :

Une première relecture, avant d'appliquer le patch et de le tester en vrai :

Dans authentic2/apps.py :

Je pense qu'on peut laisser expected_type en minuscules
et donc retirer le upper() avant l'assignation à current_type.

C'est copié/collé mais pourquoi pas.

J'ai l'impression que django.db.models.fields.Field.get_attname_column ne remonte par d'erreur si la colonne n'est pas trouvée, à la place il renvoie le nom d'attribut.

Hmm ? Je ne comprends pas, tu veux qu'il retourne quoi ?

Je serais donc vigilant lorsqu'on fait un
cursor.fetchone()[0].upper()

Dans authentic2/compat.py :

Dans le constructeur de authentic2.compat.JSONField :

[...]
→ je crois que le raise est de trop, non ? ou alors assert self.__dj11_field à la place ?

Yep debug qui traîne.

la méthode contribute_to_class doit supporter un paramètre private_only (et, respecter l'interface, son équivalent déprécié virtual_only).

C'est pour cela que j'ai mis kwargs, pour ignorer ça, mais je peux mettre un assert not kwargs pour être sûr que ça n'arrive jamais.

D'ailleurs cette méthode, dans le code django de contrib postgres se termine par un bout de code qui me paraît nécessaire aussi ici :
[...]

Pareil on va dire que ça ne doit pas arriver (définition d'un nom explicite de colonne impliquant la création d'un attribut déferré).

Dans la méthode compat.JSONField.deconstruct :
Je ne comprends ce bout de code :
[...]
Le code originel du Field django renvoie toujours un quadruplet dont le premier élément est le nom du champ. Pourquoi le retirer ici ?

Parce que sinon ça ne marche pas, voir le code de serialize et deep_deconstruct dans django.db.migrations.autodetector et autour; la méthode deconstruct des Field renvoie un quadruplet mais tous les autres objets des triplets; comme on hérite pas de Field le moteur de migration prend le champ pour un autre type d'objet.

Il faudra aussi m'expliquer d'où vienne jsonfield.fields.configure_database_connection et jsonfield.fields.connection_created.
Je ne trouve pas le code, et donc je ne peux pas comprendre ce que tu cherches à faire avec ces quelques lignes à la fin de compat.py.

Contourner le bug d'incompatibilité entre django.contrib.postgres.fiels.JSONField et jsonfield.JSONField, celui que tu as pointé.

Dans tests/settings.py :

[...]

Wut ?!

Bug tordu, le nouveau champ fait un accès à la base à l'initialisation, et donc en dehors des tests, (pour connaître la version de postgres, mais dès que tu accèdes à django.db.connection même pour lire vendor qui n'est qu'une info coté Django, ça ouvre une connection), comme les settings des tests ne définissait pas NAME ça ne passait pas; l'autre possibilité ce serait de ne pas faire cette accès si DATABASES n'est pas initialisé correctement (il l'est ensuite dans les tests). Et j'ai mis postgres parce que c'est la seule base dont on est à peu près sûr qu'elle est toujours là (mais ça pourrait foirer).

Benjamin Dauvergne a écrit :

Hmm ? Je ne comprends pas, tu veux qu'il retourne quoi ?

Ok, je me suis emmêlé les pinceaux.

C'est pour cela que j'ai mis kwargs, pour ignorer ça, mais je peux mettre un assert not kwargs pour être sûr que ça n'arrive jamais.

Et donc on ne reprend pas le bout de code :

        if private_only:                                                             
            cls._meta.add_field(self, private=True)                                  
        else:                                                                        
            cls._meta.add_field(self)     

Pareil on va dire que ça ne doit pas arriver (définition d'un nom explicite de colonne impliquant la création d'un attribut déferré).

Ok d'ac.

Parce que sinon ça ne marche pas, voir le code de serialize et deep_deconstruct dans django.db.migrations.autodetector et autour; la méthode deconstruct des Field renvoie un quadruplet mais tous les autres objets des triplets; comme on hérite pas de Field le moteur de migration prend le champ pour un autre type d'objet.

Ok, vu pour la méthode deep_deconstruct, mais pas pour serialize (pas trouvé dans le code db.migrations.autodetector de dj1.11 ni dj1.8).

Contourner le bug d'incompatibilité entre django.contrib.postgres.fiels.JSONField et jsonfield.JSONField, celui que tu as pointé.

Ok, pardon, relecture trop rapide de ma part.

Bug tordu, le nouveau champ fait un accès à la base à l'initialisation, et donc en dehors des tests, (pour connaître la version de postgres, mais dès que tu accèdes à django.db.connection même pour lire vendor qui n'est qu'une info coté Django, ça ouvre une connection), comme les settings des tests ne définissait pas NAME ça ne passait pas; l'autre possibilité ce serait de ne pas faire cette accès si DATABASES n'est pas initialisé correctement (il l'est ensuite dans les tests). Et j'ai mis postgres parce que c'est la seule base dont on est à peu près sûr qu'elle est toujours là (mais ça pourrait foirer).

Ok, est-ce qu'on peut mettre en substance ça en commentaire dans ce fichier ?

Je teste le patch maintenant.

(Et donc à toi de voir si on prend en compte le support de champs JSONField en attribut privé de la classe modèle parente.)

Paul Marillonnet a écrit :

Et donc on ne reprend pas le bout de code :
[...]

Non puisqu'on s'assurerait via assert not kwargs que ça ne peut pas arriver (ou bien ça plante, vu le moment d'initialisation très tôt des modèles, on le ratera pas).

de django.db.models.fields.Field.contribute_to_class, qui justifie la présence d'un tel paramètre private_only ?
Pourquoi est-ce qu'on ne pourrait pas avoir de champ JSON privé pour un modèle ?

Je ne répondrai que si tu peux me dire à quoi servent les champs privés/virtuels dans Django.

Pareil on va dire que ça ne doit pas arriver (définition d'un nom explicite de colonne impliquant la création d'un attribut déferré).

Ok d'ac.

Parce que sinon ça ne marche pas, voir le code de serialize et deep_deconstruct dans django.db.migrations.autodetector et autour; la méthode deconstruct des Field renvoie un quadruplet mais tous les autres objets des triplets; comme on hérite pas de Field le moteur de migration prend le champ pour un autre type d'objet.

Ok, vu pour la méthode deep_deconstruct, mais pas pour serialize (pas trouvé dans le code db.migrations.autodetector de dj1.11 ni dj1.8).

Oui c'est ailleurs, dans django.db.migrations.serializer.

Bug tordu, le nouveau champ fait un accès à la base à l'initialisation, et donc en dehors des tests, (pour connaître la version de postgres, mais dès que tu accèdes à django.db.connection même pour lire vendor qui n'est qu'une info coté Django, ça ouvre une connection), comme les settings des tests ne définissait pas NAME ça ne passait pas; l'autre possibilité ce serait de ne pas faire cette accès si DATABASES n'est pas initialisé correctement (il l'est ensuite dans les tests). Et j'ai mis postgres parce que c'est la seule base dont on est à peu près sûr qu'elle est toujours là (mais ça pourrait foirer).

Ok, est-ce qu'on peut mettre en substance ça en commentaire dans ce fichier ?

Oui tu as raison je vais faire ça, à défaut d'être compréhensible ce sera là.

Je teste le patch maintenant.

Il me semble que ça a cassé le job jenkins d'authentic2-auth-fc, https://jenkins2.entrouvert.org/job/authentic2-auth-fc/119/