https://dev.entrouvert.org/https://dev.entrouvert.org/favicon.ico?15861920342019-10-16T12:51:34ZRedmine Entr’ouvertAuthentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=1972822019-10-16T12:51:34ZPaul Marillonnet
<ul><li><strong>Fichier</strong> <a href="/attachments/38191">0001-drop-JSONField-compatibility-with-multiple-db-vendor.patch</a> <a class="icon-only icon-download" title="Télécharger" href="/attachments/download/38191/0001-drop-JSONField-compatibility-with-multiple-db-vendor.patch">0001-drop-JSONField-compatibility-with-multiple-db-vendor.patch</a> ajouté</li><li><strong>Statut</strong> changé de <i>Nouveau</i> à <i>Solution proposée</i></li><li><strong>Patch proposed</strong> changé de <i>Non</i> à <i>Oui</i></li></ul> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=1972882019-10-16T12:59:58ZBenjamin Dauvergne
<ul><li><strong>Statut</strong> changé de <i>Solution proposée</i> à <i>Rejeté</i></li></ul><p>Je ne vois pas en quoi ça simplifie le support python3, et je suis attaché au support sqlite même partiel; le mieux c'est me poser la question avant de faire un patch.</p> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=1972962019-10-16T13:16:42ZPaul Marillonnet
<ul></ul><p>Benjamin Dauvergne a écrit :</p>
<blockquote>
<p>Je ne vois pas en quoi ça simplifie le support python3,</p>
</blockquote>
<p>Solution de facilité, qui m'épargne d'implémenter des opérateurs de comparaisons entre champs.</p>
<blockquote>
<p>et je suis attaché au support sqlite même partiel; le mieux c'est me poser la question avant de faire un patch.</p>
</blockquote>
<p>Je ne savais pas. Je vais laisser de côté cette solution de facilité et porter ce bout de code en python3.</p> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=1978102019-10-19T17:57:12ZBenjamin Dauvergne
<ul></ul><p>Paul Marillonnet a écrit :</p>
<blockquote>
<p>Benjamin Dauvergne a écrit :</p>
<blockquote>
<p>Je ne vois pas en quoi ça simplifie le support python3,</p>
</blockquote>
<p>Solution de facilité, qui m'épargne d'implémenter des opérateurs de comparaisons entre champs.</p>
</blockquote>
<p>Pour ?</p> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=1980042019-10-21T16:18:07ZPaul Marillonnet
<ul></ul><p>Benjamin Dauvergne a écrit :</p>
<blockquote>
<p>Pour ?</p>
</blockquote>
<p>Le code de <code>bisect.bisect</code> qui semblerait-il a changé depuis :<br /><pre>
File "/home/paul/src/authentic/src/authentic2_auth_oidc/models.py", line 45, in <module>
class OIDCProvider(models.Model):
File "/tmp/tox-paul/authentic/py3-coverage-dj111-authentic-pg-/lib/python3.7/site-packages/django/db/models/base.py", line 162, in __new__
new_class.add_to_class(obj_name, obj)
File "/tmp/tox-paul/authentic/py3-coverage-dj111-authentic-pg-/lib/python3.7/site-packages/django/db/models/base.py", line 325, in add_to_class
value.contribute_to_class(cls, name)
File "/home/paul/src/authentic/src/authentic2/compat.py", line 90, in contribute_to_class
cls._meta.add_field(self)
File "/tmp/tox-paul/authentic/py3-coverage-dj111-authentic-pg-/lib/python3.7/site-packages/django/db/models/options.py", line 277, in add_field
self.local_fields.insert(bisect(self.local_fields, field), field)
TypeError: '<' not supported between instances of 'JSONField' and 'URLField'
ERROR: InvocationError for command /tmp/tox-paul/authentic/py3-coverage-dj111-authentic-pg-/bin/py.test --cov=src --cov-branch --cov-append --cov-report xml --cov-report html tests/ --random-group (exited with code 1)
</pre></p>
<p>Je regarde dès que j'ai un peu de temps.</p> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=1980152019-10-21T17:50:17ZBenjamin Dauvergne
<ul></ul><p>On obtient cette trace en faisant quoi ?</p> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=1981802019-10-22T11:30:32ZPaul Marillonnet
<ul></ul><p>Benjamin Dauvergne a écrit :</p>
<blockquote>
<p>On obtient cette trace en faisant quoi ?</p>
</blockquote>
<p>Directement à l'initialisation des tests<sup><a href="#fn1">1</a></sup>, après avoir bien sûr adapté le fichier tox.ini :<br /><pre><code class="diff syntaxhl"><span class="gh">diff --git a/tox.ini b/tox.ini
index 29f0921ed..34e58e60d 100644
</span><span class="gd">--- a/tox.ini
</span><span class="gi">+++ b/tox.ini
</span><span class="p">@@ -5,7 +5,7 @@</span>
[tox]
toxworkdir = {env:TMPDIR:/tmp}/tox-{env:USER}/authentic/
<span class="gd">-envlist = py27-coverage-dj111-authentic-pg-{oldldap,},py27-coverage-dj111-rbac-pg,pylint
</span><span class="gi">+envlist = {py27,py3}-coverage-dj111-authentic-pg-{oldldap,},{py3,py27}-coverage-dj111-rbac-pg,pylint
</span>
[testenv]
whitelist_externals =
<span class="p">@@ -61,7 +61,8 @@</span> deps =
ldaptools>=0.15
oldldap: python-ldap<3
commands =
<span class="gd">- ./getlasso.sh
</span><span class="gi">+ py27: ./getlasso.sh
+ py3: ./getlasso3.sh
</span> rbac,authentic: py.test {env:FAST:} {env:REUSEDB:} {env:COVERAGE:} {env:JUNIT:} {posargs:{env:TESTS} --random-group}
[testenv:pylint]
</code></pre></p>
<p>La trace complète <a href="https://perso.entrouvert.org/~pmarillonnet/tmp/py3_jsonfield_failure.html" class="external">ici</a></p> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=1981932019-10-22T12:30:03ZBenjamin Dauvergne
<ul><li><strong>Fichier</strong> <a href="/attachments/38331">JSONField_cmp.diff</a> <a class="icon-only icon-download" title="Télécharger" href="/attachments/download/38331/JSONField_cmp.diff">JSONField_cmp.diff</a> ajouté</li></ul><p>Et avec ce patch ? Si c'est ok ouvre un ticket migration python3 pour ce patch (j'ai du mal à comprendre pourquoi ça n'arrive qu'en python3).</p> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=1982252019-10-22T13:53:50ZPaul Marillonnet
<ul></ul><p>Benjamin Dauvergne a écrit :</p>
<blockquote>
<p>Et avec ce patch ? Si c'est ok ouvre un ticket migration python3 pour ce patch (j'ai du mal à comprendre pourquoi ça n'arrive qu'en python3).</p>
</blockquote>
<p><a href="https://perso.entrouvert.org/~pmarillonnet/tmp/py3_jsonfield_failure2.html" class="external">Toujours pas.</a></p> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=1982652019-10-22T15:50:30ZBenjamin Dauvergne
<ul></ul><p>Paul Marillonnet a écrit :</p>
<blockquote>
<p>Benjamin Dauvergne a écrit :</p>
<blockquote>
<p>Et avec ce patch ? Si c'est ok ouvre un ticket migration python3 pour ce patch (j'ai du mal à comprendre pourquoi ça n'arrive qu'en python3).</p>
</blockquote>
<p><a href="https://perso.entrouvert.org/~pmarillonnet/tmp/py3_jsonfield_failure2.html" class="external">Toujours pas.</a></p>
</blockquote>
<p>En fait il faudrait juste déléguer <i>eq</i> et <i>lt</i> à <code>.__real_field__()</code></p> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=1983612019-10-23T09:08:45ZPaul Marillonnet
<ul></ul><p>Après quelques modifs pour que ça tourne, par dessus ton diff :<br /><pre><code class="diff syntaxhl"><span class="gh">diff --git a/src/authentic2/compat.py b/src/authentic2/compat.py
index f7e48e49c..520de2815 100644
</span><span class="gd">--- a/src/authentic2/compat.py
</span><span class="gi">+++ b/src/authentic2/compat.py
</span><span class="p">@@ -51,16 +51,16 @@</span> class JSONField(object):
self.__kwargs = kwargs
if django.VERSION >= (1, 11):
try:
<span class="gd">- from django.contrib.postgres.fields import JSONField
- self.__dj11_field = JSONField(*args, **kwargs)
</span><span class="gi">+ from django.contrib.postgres.fields import JSONField as Dj11Field
+ self.__dj11_field = Dj11Field(*args, **kwargs)
</span> except ImportError:
pass
try:
<span class="gd">- from jsonfield.fields import JSONField
- self.__jsonfield_field = JSONField(*args, **kwargs)
</span><span class="gi">+ from jsonfield.fields import JSONField as JsonfieldField
+ self.__jsonfield_field = JsonfieldField(*args, **kwargs)
</span> except ImportError:
pass
<span class="gd">- self.creation_counter = Field.creation_counter
</span><span class="gi">+ super(JSONField, self).__setattr__('creation_counter', Field.creation_counter)
</span> Field.creation_counter += 1
def __real_field__(self):
</code></pre></p>
<p>on tombe sur une <a href="https://perso.entrouvert.org/~pmarillonnet/tmp/py3_jsonfield_failure3.html" class="external">trace qui laisse entendre qu'il va falloir un peu plus de boulot.</a><br />(i.e. le <code>authentic2.compat.JSONField</code> en opérande de comparaison à droite.)</p>
<p>Je n'ai pas d'idée pour l'instant, je creuse l'affaire dès que j'ai un peu de temps.</p> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=1985942019-10-24T14:03:41ZPaul Marillonnet
<ul><li><strong>Sujet</strong> changé de <i>Retirer la compatibilité JSONField pour SQLite et Django < 1.11</i> à <i>python3 : porter authentic2.compat.JSONField</i></li></ul> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=1985952019-10-24T14:03:51ZPaul Marillonnet
<ul><li><strong>Statut</strong> changé de <i>Rejeté</i> à <i>En cours</i></li></ul> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=1987822019-10-25T14:42:00ZPaul Marillonnet
<ul></ul><p>Après réflexion je pense que le plus simple est d'altérer le comportement du builtin <code>isinstance</code> au moment de la définition des modèles qui ont un champ de type <code>authentic2.compat.JSONField</code>, et ceci afin de leurrer la méthode <a href="https://github.com/django/django/blob/stable/1.11.x/django/db/models/fields/__init__.py#L477" class="external"><code>__lt__</code> de la classe native Field de django</a> à l'origine de l'erreur lors de la tentative de comparaison à droite.</p>
<p>Peut-être un gestionnaire de contexte qui ferait un truc du genre :<br /><pre><code class="python syntaxhl"><span class="kn">from</span> <span class="nn">django.utils.six.moves</span> <span class="kn">import</span> <span class="n">builtins</span> <span class="k">as</span> <span class="n">__builtin__</span>
<span class="o">@</span><span class="n">contextmanager</span>
<span class="k">def</span> <span class="nf">fake_isinstance_contextmanager</span><span class="p">():</span>
<span class="n">isinstance_orig</span> <span class="o">=</span> <span class="n">__builtin__</span><span class="p">.</span><span class="nb">isinstance</span>
<span class="k">def</span> <span class="nf">fake_isinstance</span><span class="p">(</span><span class="n">obj</span><span class="p">,</span> <span class="n">clz</span><span class="p">):</span>
<span class="k">if</span> <span class="n">isinstance_orig</span><span class="p">(</span><span class="n">obj</span><span class="p">,</span> <span class="n">compat</span><span class="p">.</span><span class="n">JSONField</span><span class="p">)</span> <span class="ow">and</span> <span class="nb">issubclass</span><span class="p">(</span><span class="n">clz</span><span class="p">,</span> <span class="n">models</span><span class="p">.</span><span class="n">Field</span><span class="p">):</span>
<span class="k">return</span> <span class="bp">True</span>
<span class="k">else</span><span class="p">:</span>
<span class="k">return</span> <span class="n">isinstance_orig</span><span class="p">(</span><span class="n">obj</span><span class="p">,</span> <span class="n">clz</span><span class="p">)</span>
<span class="k">try</span><span class="p">:</span>
<span class="n">__builtin__</span><span class="p">.</span><span class="nb">isinstance</span> <span class="o">=</span> <span class="n">fake_isinstance</span>
<span class="k">yield</span>
<span class="k">finally</span><span class="p">:</span>
<span class="n">__builtin__</span><span class="p">.</span><span class="nb">isinstance</span> <span class="o">=</span> <span class="n">isinstance_orig</span>
<span class="k">with</span> <span class="n">fake_isinstance_contextmanager</span><span class="p">():</span>
<span class="k">class</span> <span class="nc">OIDCProvider</span><span class="p">(</span><span class="n">models</span><span class="p">.</span><span class="n">Model</span><span class="p">):</span>
<span class="c1"># […]
</span></code></pre></p> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=1991832019-10-29T14:43:12ZPaul Marillonnet
<ul></ul><p>Benj, en réunion dev :</p>
<blockquote>
<p>je pense qu'on va dumper, j'abandonne</p>
</blockquote>
<p>Je pense qu'on peut en profiter pour délaisser la dépendance à django-jsonfield et se baser sur le champ natif postgresql de django.</p> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=1992962019-10-29T20:15:31ZBenjamin Dauvergne
<ul></ul><p>Tu peux reprendre ton premier patch et y ajouter de virer tout ce qui concerne sqlite dans les tests.</p> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=2014272019-11-15T16:44:30ZPaul Marillonnet
<ul><li><strong>Fichier</strong> <a href="/attachments/39016">0001-Revert-add-compatibility-layer-for-support-of-Django.patch</a> <a class="icon-only icon-download" title="Télécharger" href="/attachments/download/39016/0001-Revert-add-compatibility-layer-for-support-of-Django.patch">0001-Revert-add-compatibility-layer-for-support-of-Django.patch</a> ajouté</li><li><strong>Fichier</strong> <a href="/attachments/39015">0002-auth_oidc-drop-now-redundant-django-jsonfield-depend.patch</a> <a class="icon-only icon-download" title="Télécharger" href="/attachments/download/39015/0002-auth_oidc-drop-now-redundant-django-jsonfield-depend.patch">0002-auth_oidc-drop-now-redundant-django-jsonfield-depend.patch</a> ajouté</li><li><strong>Fichier</strong> <a href="/attachments/39017">0003-tests-drop-partial-sqlite-support-36990.patch</a> <a class="icon-only icon-download" title="Télécharger" href="/attachments/download/39017/0003-tests-drop-partial-sqlite-support-36990.patch">0003-tests-drop-partial-sqlite-support-36990.patch</a> ajouté</li><li><strong>Statut</strong> changé de <i>En cours</i> à <i>Solution proposée</i></li></ul><p>Sans doute comme ça. Dans le premier patch je passe par un revert tout bête qui réintroduit la dépendance django-jsonfield, pour la virer juste après, dans le second patch. Est-ce que c'est grave docteur ? :)</p>
<p>Autre question que je me pose : aurait-on intérêt à tester que le patch modifiant la migration ne va pas casser les champs jusque-là stockés sous forme de <code>jsonfield.JSONField</code>, et qui à l'insu de leur plein gré deviennent des <code>django.contrib.postgres.fields.JSONField</code> ?</p> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=2024942019-11-21T16:14:10ZPaul Marillonnet
<ul><li><strong>Fichier</strong> <a href="/attachments/39212">0001-Revert-add-compatibility-layer-for-support-of-Django.patch</a> <a class="icon-only icon-download" title="Télécharger" href="/attachments/download/39212/0001-Revert-add-compatibility-layer-for-support-of-Django.patch">0001-Revert-add-compatibility-layer-for-support-of-Django.patch</a> ajouté</li><li><strong>Fichier</strong> <a href="/attachments/39211">0002-auth_oidc-drop-now-redundant-django-jsonfield-depend.patch</a> <a class="icon-only icon-download" title="Télécharger" href="/attachments/download/39211/0002-auth_oidc-drop-now-redundant-django-jsonfield-depend.patch">0002-auth_oidc-drop-now-redundant-django-jsonfield-depend.patch</a> ajouté</li><li><strong>Fichier</strong> <a href="/attachments/39213">0003-tests-drop-partial-sqlite-support.patch</a> <a class="icon-only icon-download" title="Télécharger" href="/attachments/download/39213/0003-tests-drop-partial-sqlite-support.patch">0003-tests-drop-partial-sqlite-support.patch</a> ajouté</li></ul><p>Paul Marillonnet a écrit :</p>
<blockquote>
<p>Autre question que je me pose : aurait-on intérêt à tester que le patch modifiant la migration ne va pas casser les champs jusque-là stockés sous forme de <code>jsonfield.JSONField</code>, et qui à l'insu de leur plein gré deviennent des <code>django.contrib.postgres.fields.JSONField</code> ?</p>
</blockquote>
<p>En ayant relu le code de django-jsonfield et du code de contrib de django, je suis rassuré.<br />On est sur du pg>9.4 sur Debian Stretch, donc la situation où des données jusque là stockées en type pg 'TEXT' ou 'JSON' seraient interprétées comme du 'JSONB' suite à la migration ne se posera pas (et quand bien même elle se poserait, il y a l'intelligence dans psycopg2 pour faire face à ce genre de situation<sup><a href="#fn1">1</a></sup>).</p>
<p>Je remets donc sur la table les trois patches (quasiment) inchangés.</p>
<p id="fn1" class="footnote"><sup>1</sup> <a class="external" href="https://github.com/psycopg/psycopg2/blob/master/lib/_json.py#L158">https://github.com/psycopg/psycopg2/blob/master/lib/_json.py#L158</a></p> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=2024952019-11-21T16:19:18ZBenjamin Dauvergne
<ul><li><strong>Statut</strong> changé de <i>Solution proposée</i> à <i>Solution validée</i></li></ul> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=2025002019-11-21T16:46:56ZPaul Marillonnet
<ul><li><strong>Statut</strong> changé de <i>Solution validée</i> à <i>Résolu (à déployer)</i></li></ul><pre>
commit 2cc2cf4200b69ddc36b6262456b7afbadfa0d963
Author: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Fri Nov 15 17:16:50 2019 +0100
tests: drop partial sqlite support
commit 6d064a2c223e900d670b398578e971f73aea6a8b
Author: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Fri Nov 15 17:01:36 2019 +0100
auth_oidc: drop now-redundant django-jsonfield dependency
commit cef04caf83376793c29cb58d4bc0fc690b251762
Author: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Tue Oct 29 11:18:16 2019 +0100
Revert "add compatibility layer for support of Django native JSONField (fixes #29193)"
This reverts commit d730dba525215d615fa2b974d44183d16909f4d2.
</pre> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=2025012019-11-21T16:49:25ZPaul Marillonnet
<ul></ul><p>Et j'avais les bons messages de commit dans une branche locale, que j'ai écrasée par erreur sans m'en rendre compte. Mes excuses pour ces messages de commit qui ne font pas apparaître le numéro de ticket. :/</p> Authentic 2 - Development #36990: python3 : porter authentic2.compat.JSONFieldhttps://dev.entrouvert.org/issues/36990?journal_id=2157972020-02-15T19:18:46ZFrédéric Pétersfpeters@entrouvert.com
<ul><li><strong>Statut</strong> changé de <i>Résolu (à déployer)</i> à <i>Solution déployée</i></li></ul>