Project

General

Profile

Development #46476

Ne plus dépendre de pkg_resources

Added by Emmanuel Cazenave over 2 years ago. Updated almost 2 years ago.

Status:
Solution proposée
Priority:
Normal
Target version:
-
Start date:
08 September 2020
Due date:
% Done:

0%

Estimated time:
Patch proposed:
Yes
Planning:
No

Description

En se basant sur #46474 plus de la déclaration explicite dans les settings.


Files


Related issues

Related to Authentic 2 - Development #22865: Ne plug utiliser pkg_resources pour charger les applications incluses dans authenticEn cours28 March 2018

Actions

History

#1

Updated by Emmanuel Cazenave over 2 years ago

  • Related to Development #22865: Ne plug utiliser pkg_resources pour charger les applications incluses dans authentic added
#2

Updated by Emmanuel Cazenave over 2 years ago

Et donc ça suppose de rajouter deux lignes dans les settings pour l'activer maintenant, sur l'idée de respecter les indications de la doc django https://docs.djangoproject.com/en/1.11/topics/settings/#altering-settings-at-runtime : "The only place you should assign to settings is in a settings file."

#3

Updated by Frédéric Péters over 2 years ago

Quel comportement s'il y a ajout aujourd'hui de quelque chose dans les settings ?

Et vraiment, s'il faut vraiment taper :

  import authentic2_auth_fedict

  INSTALLED_APPS, AUTHENTICATION_BACKENDS, AUTH_FRONTENDS = authentic2_auth_fedict.register(
      INSTALLED_APPS, AUTHENTICATION_BACKENDS, AUTH_FRONTENDS
  )

Ça va juste pas trop marcher posé dans un settings.d/whatever.py, il me semble, s'il y a plusieurs "plugins" à charger ainsi ?

Il y a pas juste moyen d'avoir l'ajout à INSTALLED_APPS et derrière dans un app::ready() gérer quelque chose ?

#4

Updated by Emmanuel Cazenave over 2 years ago

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

Quel comportement s'il y a ajout aujourd'hui de quelque chose dans les settings ?

Pas compris.

Ça va juste pas trop marcher posé dans un settings.d/whatever.py, il me semble, s'il y a plusieurs "plugins" à charger ainsi ?

Je ne vois pas pourquoi ça ne marcherait pas, j'ai testé ça avec un autre plugin en plus déclaré de la même manière dans settings.d/xx et ça me semble ok (ça ne plante pas au démarrage, plus quelques vérifs dans un shell sur la config)

Il y a pas juste moyen d'avoir l'ajout à INSTALLED_APPS et derrière dans un app::ready() gérer quelque chose ?

C'est peut-être possible mais pas casher, ou alors tu pense à autre chose que la chose suivante (extrait de la doc) ?

from django.conf import settings

settings.DEBUG = True   # Don't do this!
#5

Updated by Frédéric Péters over 2 years ago

Pas compris.

Si je copie/colle les lignes qu'il faudra dans un fichier, dès maintenant, est-ce que ça va casser le fonctionnement actuel ?

Je ne vois pas pourquoi ça ne marcherait pas.

J'imaginais des problèmes de scopes, avec les variables modifiées ne "remontant" pas au niveau du settings.py, mais ok si les appels successifs à exec() donnent un truc ok.

C'est peut-être possible mais pas casher, ou alors tu pense à autre chose que la chose suivante (extrait de la doc) ?

Je pense à quelque chose qui serait de l'ordre de :

class ... AppConfig ...:
    def ready(self):
        register_authentication_backend(...)
        register_authentication_frontend(...)

comme on peut avoir dans combo

    def ready(self):
        engines.register(self.get_search_engines)
#6

Updated by Benjamin Dauvergne over 2 years ago

Je suis d'accord avec Manu, si on déprécie l'initialisation via pkg_resources qui a lieu avant celle de Django on a pas de moyen stable d'altérer settings.INSTALLED_APPS depuis un objet AppConfig (ça se mord la queue, trouver les AppConfig dépend de INSTALLED_APPS on ne peut pas y toucher ensuite). Coté authentic2-cut ça va m'obliger à déclarer explicitement authentic2_cut, sorl.thumbnail et le middleware pour avoir les données des partenaires dans settings.d.

C'est peut-être mieux ainsi que tout soit explicite.

#7

Updated by Frédéric Péters over 2 years ago

Quand j'écris :

Il y a pas juste moyen d'avoir l'ajout à INSTALLED_APPS ?

Je suis tout à fait d'accord pour modifier INSTALLED_APPS(/TENANT_APPS) dans un settings.d/whatever.py; le truc qui m'ennuie est de devoir exécuter du code dans ce fichier,

import authentic2_auth_fedict

INSTALLED_APPS, AUTHENTICATION_BACKENDS, AUTH_FRONTENDS = authentic2_auth_fedict.register(
      INSTALLED_APPS, AUTHENTICATION_BACKENDS, AUTH_FRONTENDS
  )
#8

Updated by Benjamin Dauvergne over 2 years ago

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

Je pense à quelque chose qui serait de l'ordre de :

[...]

On peut toucher je pense sans que ce soit dangereux (parce qu'il n'y a pas de cache lié à ma connaissance) :
  • MIDDLEWARE
  • AUTHENTICATION_BACKENDS
  • les settings d'A2 en général

Mais le souci c'est ça :

class MultitenantAppConfig(AppConfig):
    name = 'hobo.multitenant'
    verbose_name = 'Multitenant'

    def ready(self):
        from django.db.migrations import operations
        from django.db import migrations
        from django import conf

        # Install tenant aware settings
        if not isinstance(conf.settings._wrapped,
                          settings.TenantSettingsWrapper):
            conf.settings._wrapped = settings.TenantSettingsWrapper(
                conf.settings._wrapped)
            # reset settings getattr method to a cache-less version, to cancel
            # https://code.djangoproject.com/ticket/27625.
            conf.LazySettings.__getattr__ = lambda self, name: getattr(self._wrapped, name)
        threads.install_tenant_aware_threads()

ces modifications doivent avoir lieu avant que les settings multitenant ne soient installés, sinon ça ne modifiera pas les settings "globaux".

#9

Updated by Benjamin Dauvergne over 2 years ago

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

Quand j'écris :

Il y a pas juste moyen d'avoir l'ajout à INSTALLED_APPS ?

Je suis tout à fait d'accord pour modifier INSTALLED_APPS(/TENANT_APPS) dans un settings.d/whatever.py; le truc qui m'ennuie est de devoir exécuter du code dans ce fichier,

[...]

Ben ce sera plus verbeux et à documenter dans le wiki du plugin :

# ajouter dans /etc/authentic2/settins.d/authentic-gnm.py

INSTALLED_APPS += ('authentic2_gnm',)
TENANT_APPS += ('authentic2_gnm',)
MIDDLEWARE += ('authentic2_gnm.middlewares.WhateverMiddleware',)
AUTHENTICATION_BACKENDS += etc...

Pour tout ce qui est "Django" (middleware / apps / auth-bakcends) on ne peut pas faire autrement que de modifier les settings à un moment. Pour ce qui est pure A2 (notamment AUTH_FRONTENDS) on peut aussi ne plus passer du tout par les settings et demander directement aux AppConfig à chaque fois.

#10

Updated by Frédéric Péters over 2 years ago

Bon je vais m'en foutre, juste besoin de savoir quel code copier/coller dans un settings.d/whatever.py et que ce code puisse déjà y être mis, avant ces changements.

#11

Updated by Emmanuel Cazenave over 2 years ago

Benjamin Dauvergne a écrit :

Pour ce qui est pure A2 (notamment AUTH_FRONTENDS) on peut aussi ne plus passer du tout par les settings et demander directement aux AppConfig à chaque fois.

Pour AUTH_FRONTENDS et IDP_BACKENDS, l'ordre de déclaration n'importe pas ? Ce qui ouvre la porte à passer par les AppConfig, plugin whatever ?

#12

Updated by Emmanuel Cazenave over 2 years ago

Bon, ma question précédente dans l'idée de voir ce qu'on pourrait faire coté a2 pour permettre qu'un plugin externe n'ai besoin que de se rajouter dans INSTALLED_APPS, ce qui me plairait aussi, mais je ne vois rien d'évident, et de toute façon on ne peut pas contourner la déclaration dans AUTHENTICATION_BACKENDS (ailleurs ce sera MIDDLEWARE).

Je vais en rester à se passer de pkg_resource pour ce ticket, l'activation redevient explicite, c'est déjà un pas en avant.

Version actualisée du patch (le premier oubliais de gérer TENANT_APPS).

J'ai vérifié qu' on pouvait poser dès maintenant un

import authentic2_auth_fedict

if hasattr(authentic2_auth_fedict, 'register'):
    INSTALLED_APPS, TENANT_APPS, AUTHENTICATION_BACKENDS, AUTH_FRONTENDS = authentic2_auth_fedict.register(
        INSTALLED_APPS, TENANT_APPS, AUTHENTICATION_BACKENDS, AUTH_FRONTENDS
    )

Ça me semble ok.

Aussi vérifié qu'on pouvait poser ce genre de code pour deux plugins différents dans deux fichiers settings.d/xxx différents et ça me semble ok aussi (j'avais testé uniquement plusieurs déclarations successives dans un seul fichier settings.d/plugin.py).

Voilà pour les objections pratiques.

#13

Updated by Frédéric Péters over 2 years ago

J'ai vérifié qu' on pouvait poser dès maintenant un

Dernière vérification de mon côté : si c'est posé, que le module authentic2-fedict est mis à jour mais qu'authentic n'a pas encore #22865, ça passe ?

#14

Updated by Benjamin Dauvergne over 2 years ago

Emmanuel Cazenave a écrit :

Benjamin Dauvergne a écrit :

Pour ce qui est pure A2 (notamment AUTH_FRONTENDS) on peut aussi ne plus passer du tout par les settings et demander directement aux AppConfig à chaque fois.

Pour AUTH_FRONTENDS et IDP_BACKENDS, l'ordre de déclaration n'importe pas ? Ce qui ouvre la porte à passer par les AppConfig, plugin whatever ?

De toute façon avec pkg_resources on avait pas un ordre particulier il me semble, en tout cas j'en ai jamais dépendu, j'ajouter les trucs soit à la fin soit au début d'un setting (généralement à la fin sauf pour les url_patterns) mais dans quel ordre, je n'en savais rien.

#15

Updated by Emmanuel Cazenave over 2 years ago

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

J'ai vérifié qu' on pouvait poser dès maintenant un

Dernière vérification de mon côté : si c'est posé, que le module authentic2-fedict est mis à jour mais qu'authentic n'a pas encore #22865, ça passe ?

J'imagine que tu voulais dire #46474.
Et donc authentic sans #46474 et authentic2-fedict avec ce patch + les lignes de conf, ça va complètement foirer.

#16

Updated by Frédéric Péters over 2 years ago

J'imagine que tu voulais dire #46474.

Non je voulais dire le ticket que je pointais qui était celui que j'imaginais entrer en compte.

Mais donc le code que je souhaiterais poser dans les settings c'est du code qui :

  • fonctionne aujourd'hui, sans le patch de ce ticket, avec la version d'authentic de production
  • fonctionne demain, avec le patch de ce ticket, avec la version d'authentic de production
  • fonctionne après la prochaine mise à jour, avec le patch de ce ticket, avec la prochaine version d'authentic de production.

(je crois comprendre que ce n'est pas le cas).

#17

Updated by Emmanuel Cazenave over 2 years ago

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

#22865 est devenu un ticket chapeau qui ne sera fermé que quand plus aucun plugin de dépendra de pkg_resources.
#46474 est une étape dans a2 qui permet de charger les plugins via le registre des applications django, en plus de pk_resources, pour permettre une phase de transition.

Mais donc le code que je souhaiterais poser dans les settings c'est du code qui :

  • fonctionne aujourd'hui, sans le patch de ce ticket, avec la version d'authentic de production

C'est OK.

  • fonctionne demain, avec le patch de ce ticket, avec la version d'authentic de production

Le patch de ce ticket retire la déclaration de l'entry point setuptools au profit du petit bout de code qui va bien pour être découvert via #46474, donc #46474 est un pré-requis. On ne peut pas se contenter d'attendre un authentic à jour sur leur prod avant de pousser le patch qui nous occupe ici ?

  • fonctionne après la prochaine mise à jour, avec le patch de ce ticket, avec la prochaine version d'authentic de production.

c'est OK.

#18

Updated by Frédéric Péters over 2 years ago

On ne peut pas se contenter d'attendre un authentic à jour sur leur prod avant de pousser le patch qui nous occupe ici ?

Ok et donc "phase de transition" ça dit bien que donc 1/ on attend jeudi d'une mise à jour, 2/ on fait le commit à leur dépôt de construction d'image pour ajouter les settings, 3/ sans contrainte horaire ce ticket peut être validé/poussé ?

#19

Updated by Emmanuel Cazenave over 2 years ago

On du mal à se comprendre peut-être parce que je sais peu de chose sur leur façon et fréquence de mise jour (les même jeudi soir que nous ?).

Le commit sur leur fichier de confs peut-être poussé dès maintenant, ça ne cassera aucun environnement, parce tant que le patch qui nous occupe ici n'est pas poussé, ça ne fera rien :

import authentic2_auth_fedict

if hasattr(authentic2_auth_fedict, 'register'):
    INSTALLED_APPS, TENANT_APPS, AUTHENTICATION_BACKENDS, AUTH_FRONTENDS = authentic2_auth_fedict.register(
        INSTALLED_APPS, TENANT_APPS, AUTHENTICATION_BACKENDS, AUTH_FRONTENDS
    )

Ensuite ne pas pousser le patch de ce ticket tant que ça peut déboucher à être utilisé quelque part chez IMIO dans en environnement avec un a2 sans #46474.

Une fois que la possibilité d'un a2 sans #46474 a été écartée, ce patch peut-être poussé.

Plus rien à faire ensuite.

#20

Updated by Frédéric Péters over 2 years ago

On du mal à se comprendre peut-être parce que je sais peu de chose sur leur façon et fréquence de mise jour (les même jeudi soir que nous ?).

Ils produisent des images à différentes dates. Ces images tirent depuis nos dépôts de production + hotfix + pour certains modules, eobuilder, et c'est le cas pour ce module. Ça fait donc que ce commit ne peut pas être poussé avant la mise à disposition dans le dépôt de prod de l'authentic avec le commit pointé.

#21

Updated by Benjamin Dauvergne almost 2 years ago

Est-ce que quelqu'un sait si le bout de settings pointé a été transmis à IMIO ?

#22

Updated by Emmanuel Cazenave almost 2 years ago

No se.

Also available in: Atom PDF