Python Entrouvert

• refaire le middleware tenant_schemas/middleware.py qui dépend explicitement de get_tenant_model(),
• refaire les commandes migrate_schemas et sync_schemas qui parcourent la liste des schémas via un get_tenant_model().objects.all().

La commande tenant_command par contre ne dépend du modèle et peut-être utiliser telle quelle. Elle permet d'appeler n'importe quelle commande dans le contexte d'un tenant.

Côté portail admin, ça veut aussi dire refaire les formulaires de création/édition d'un tenant, pour ne plus dépendre de ModelForm, pareil pour les vues associées.

Je ne crois pas qu'on soit obligé, coté portail-admin, de faire pareil.

Benjamin Dauvergne a écrit :

La commande tenant_command par contre ne dépend du modèle et peut-être utiliser telle quelle. Elle permet d'appeler n'importe quelle commande dans le contexte d'un tenant.

En fait j'ai tort il y a aussi un problème: les commandes seront bien appelé dans le contexte de la base au niveau base de donnée mais concernant les settings comme on est pas passé dans le middleware des settings, ce n'est pas la bonne version qui en sera chargé donc il faudrait aussi prévoir au niveau des commandes le chargement des settings.

Ça serait bien que ça arrive rapidement, c'est notamment un préalable à l'installation d'un passerelle en SaaS, ce qui serait utile pour Vincennes.

Benjamin Dauvergne a écrit :

Finalement je complète/modifie ce qui est déjà dans python-entrouvert plutôt que de tout mettre dans hobo, hobo c'est finalement une application particulière.

Ok; on déplace ce ticket ou tu crées un nouveau ?

J'hésite à remplace <domain> un peu partout par le nom directement du schéma et déplacer le mapping nom de domain->schéma dans la configuration des virtual host en utilisant un entête HTTP maison nommé X-Tenant:. Dans ce cas au lieu d'un fichier /etc/logiciel/tenants/<domain>/schema qui contient le nom du schéma on a directement @/etc/logiciel/tenants/<schema>/ et le mapping des domain vers les schémas se fait dans nginx.

Qu'en pensez vous ?

Le portail admin il a le nom de domaine comme info; de là l'agent peut lire os.path.join(TENANT_BASE, <domain>, 'schema'), c'est facile.

Dans l'autre option, à partir du nom de domaine, pour trouver le tenant associé, tu verrais ça comment ? (j'ai quelques idées mais rien de bien propre)

Je suis en train de me dire que pour simplifier tout ça je pourrais avoir un simple TENANT_DIRS qui configure tout à la fois, les tenants, leur configuration et leur templates.

On aurait donc:

TENANT_DIRS = ('/etc/<logiciel>/tenants/',
'/var/lib/<logiciel>/tenants/',
'/usr/share/<logiciel>/tenants/')

Oui, avoir une fonction déterminée pour faire la correspondance domaine→tenant, ça me va très bien.

Ca me dérange vraiment qu'on se retrouve avec un nom de schéma généré depuis le hostname. Ca semble pratique à première vu mais je pense que ça va nous poser des problèmes pour les différents déploiements en prod, test et dev. On ne pourra pas copier les bases facilement et il faudra générer plein de symlinks dans les paquets pour les différents hostname (dev, test et prod).
On pourrait utiliser la technique de Benj avec un header HTTP ou alors on pourrait imaginer de mettre un mapping hostname -> nom du schéma / tenant dans un fichier de configuration générique ou encore dans chaque dossier de conf de chaque schéma on ajouterait une entrée hostnames.

Jérôme Schneider a écrit :

Ca me dérange vraiment qu'on se retrouve avec un nom de schéma généré depuis le hostname. Ca semble pratique à première vu mais je pense que ça va nous poser des problèmes pour les différents déploiements en prod, test et dev. On ne pourra pas copier les bases facilement et il faudra générer plein de symlinks dans les paquets pour les différents hostname (dev, test et prod).
On pourrait utiliser la technique de Benj avec un header HTTP ou alors on pourrait imaginer de mettre un mapping hostname -> nom du schéma / tenant dans un fichier de configuration générique ou encore dans chaque dossier de conf de chaque schéma on ajouterait une entrée hostnames.

• concernant les templates et leurs paquets, utiliser des liens symboliques avec les différents hostname de prod, preprod, etc...
• pour les migrations de données, penser à faire un DROP DATABASE <prod>; ALTER DATABASE <preppod> RENAME TO <prod> et inversement pour la reprise en preprod des données de prod.

Je notais :

avoir une fonction déterminée pour faire la correspondance domaine→tenant, ça me va très bien

Ça peut très bien être mapping.get(hostname, hostname.replace('.', '_').replace('-', '_')), et que mapping vienne d'un json posé quelque part (ou tout autre format), non ?

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

Je notais :

avoir une fonction déterminée pour faire la correspondance domaine→tenant, ça me va très bien

Ça peut très bien être mapping.get(hostname, hostname.replace('.', '_').replace('-', '_')), et que mapping vienne d'un json posé quelque part (ou tout autre format), non ?

C'est une très bonne solution. C'est ma préféré pour le moment.

Après une petite remarque pour Benjamin : je n'aime pas spécialement le fait d'avoir deux formats de fichier de configuration (json et python en l’occurrence) qui configure la même chose. Si on ne pouvait en garder qu'un ça serait parfait. Le portail citoyen v2 actuel qui prend yaml, json, python et les variables d’environnements c'est vraiment une cause de confusion. (je sais on en avait déjà parlé mais je préfère en remettre une couche).

On en a déja parlé effectivement et on a conclu qu'on ne faisait plus que JSON et Python, ce que je fais là.

Benjamin Dauvergne a écrit :

Je suis en train de me dire que pour simplifier tout ça je pourrais avoir un simple TENANT_DIRS qui configure tout à la fois, les tenants, leur configuration et leur templates.
On aurait donc: TENANT_DIRS = ('/etc/<logiciel>/tenants/', '/var/lib/<logiciel>/tenants/', '/usr/share/<logiciel>/tenants/')

Moi je mettrais le /var en dernier, mais sinon, ça me plait bien.

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

Je notais :

avoir une fonction déterminée pour faire la correspondance domaine→tenant, ça me va très bien

Ça peut très bien être mapping.get(hostname, hostname.replace('.', '_').replace('-', '_')), et que mapping vienne d'un json posé quelque part (ou tout autre format), non ?

Oui, un explicite /etc/<logiciel>/tenant_by_domain.json (et si ce fichier n'existe pas, c'est pas grave).

Ensuite, pour toute les mécaniques devop, il faudra qu'on s'ajoute des petites commandes du style manage.py get_tenant_by_domain <domain> qui nous dira le nom du tenant sans qu'on ait besoin de le chercher "à la main".

Benjamin Dauvergne a écrit :

• si os.path.join(TENANT_BASE, <domain>, 'settings.json' ou 'settings.py') existe ils sont chargés (l'ordre dépend de l'ordre de chargement des deux middleware concernés)
  (...)
  Qu'en pensez vous ?

Pour moi, faire en sorte que le settings.py soit toujours lancé après la prise en charge des settings.json

Thomas Noël a écrit :

Benjamin Dauvergne a écrit :

Je suis en train de me dire que pour simplifier tout ça je pourrais avoir un simple TENANT_DIRS qui configure tout à la fois, les tenants, leur configuration et leur templates.
On aurait donc: TENANT_DIRS = ('/etc/<logiciel>/tenants/', '/var/lib/<logiciel>/tenants/', '/usr/share/<logiciel>/tenants/')

Moi je mettrais le /var en dernier, mais sinon, ça me plait bien.

Finalement je n'ai pas suivi cette piste parce que combiné des settings depuis plusieurs répertoires tout en gérant du cache c'était compliqué, j'y repenserai si le besoin s'en fait sentir.

Thomas Noël a écrit :

Oui, un explicite /etc/<logiciel>/tenant_by_domain.json (et si ce fichier n'existe pas, c'est pas grave).

Finalement c'est un dico dans les settings nommé TENANT_MAPPING.

Thomas Noël a écrit :

Benjamin Dauvergne a écrit :

• si os.path.join(TENANT_BASE, <domain>, 'settings.json' ou 'settings.py') existe ils sont chargés (l'ordre dépend de l'ordre de chargement des deux middleware concernés)
  (...)
  Qu'en pensez vous ?

Pour moi, faire en sorte que le settings.py soit toujours lancé après la prise en charge des settings.json

Ça dépend de l'ordre dans lequel on déclare le PythonSettingsMiddleware et le JSONSettingsMiddleware donc oui c'est faisable.

En vrac.

[...] ce patch aux settings de celui-ci

On est d'accord pour dire que la config en tenants, elle vient par dessus nos applications (quand elles sont déployées, donc a priori dans le paquet .deb), mais qu'on ne modifie pas celles-ci pour l'utiliser dans leurs configs par défaut ?

 if DEBUG:
     TEMPLATE_LOADERS = (
+        'entrouvert.djommon.multitenant.template_loader.FilesystemLoader',

(sans avoir la totalité du fichier, ça fait bizarre d'avoir ça sous un "if DEBUG:")

_~~

Une mise à jour de python-entrouvert sur un serveur avec des applications utilisant les tenants, ça va passer sans peine ? (on s'en fout si on n'a aucune installation utilisant des tenants pour le moment, c'est juste que je ne suis pas sûr de ça)

_~~

0008-Import-django-tenant-schemas-commands-to-adapt-them-.patch, c'est sympa d'ajouter une petite note en haut des fichiers pour rappeler leur origine.

Et tant qu'à les adapter, je vois notamment ce bout qui pourrait aussi l'être :

        if not all_tenants:
            raise CommandError("""There are no tenants in the system.
To learn how create a tenant, see:
https://django-tenant-schemas.readthedocs.org/en/latest/use.html#creating-a-tenant""")

_~~

$ mkdir /tmp/tenants/test_localhost
$ authentic2/run.sh create_schema

En traduisant ça vers du code générique pour le portail admin, ça donnerait :

$tenant = $app_manage get_tenant_by_domain $host
mkdir /var/lib/$app/$tenant
echo XXX > /var/lib/$app/$tenant/settings.json
$app_manage create_schema

Correct ?

_~~

Finalement c'est un dico dans les settings nommé TENANT_MAPPING.

Je pense que j'aurais préféré un fichier indépendant, que ça aurait été plus simple le jour où on voudra pouvoir ajouter un tenant sans redémarrer l'application.

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

En vrac.

[...] ce patch aux settings de celui-ci

On est d'accord pour dire que la config en tenants, elle vient par dessus nos applications (quand elles sont déployées, donc a priori dans le paquet .deb), mais qu'on ne modifie pas celles-ci pour l'utiliser dans leurs configs par défaut ?

[...]

(sans avoir la totalité du fichier, ça fait bizarre d'avoir ça sous un "if DEBUG:")

J'ai modifié les settings d'authentic pour le test pas pour le commiter, et je l'ai recopié ici pour référence.

Une mise à jour de python-entrouvert sur un serveur avec des applications utilisant les tenants, ça va passer sans peine ? (on s'en fout si on n'a aucune installation utilisant des tenants pour le moment, c'est juste que je ne suis pas sûr de ça)

• la classe Tenant que j'ai rendu inopérante pour ne pas se retrouver avec des modèles créés quand on n'en veut pas
• la classe EOTenantMiddleware qui chargeait des settings depuis la base
• restauré l'ancien FilesystemLoader qui n'ajoutait pas templates/ à la fin du chemin des templates pour un tenant et utilisait MULTITENANT_TEMPLATE_DIRS comme variable et pas TENANT_TEMPLATE_DIRS, mais dans ce cas il faudrait que je créé un autre classe avec un nom différent pour le nouvelle version

_~~

0008-Import-django-tenant-schemas-commands-to-adapt-them-.patch, c'est sympa d'ajouter une petite note en haut des fichiers pour rappeler leur origine.

Ok

Et tant qu'à les adapter, je vois notamment ce bout qui pourrait aussi l'être :

[...]

Je n'ai pas vraiment de doc sur laquelle pointer pour l'instant mais je vais mettre un commentaire du style "create a directory in settings.TENANT_BASE".

_~~

[...]

En traduisant ça vers du code générique pour le portail admin, ça donnerait :

[...]

Correct ?

Non mais je vais corriger get_tenant_by_domain, pour l'instant il ne retourne la bonne valeur que si le répertoire du tenant existe déjà.

_~~

Finalement c'est un dico dans les settings nommé TENANT_MAPPING.

Je pense que j'aurais préféré un fichier indépendant, que ça aurait été plus simple le jour où on voudra pouvoir ajouter un tenant sans redémarrer l'application.

Les settings sont relus sur un kill -HUP du process gunicorn parent. Je ne suis pas fan de multiplier les fichiers de configuration, surtout qu'on peut simuler ça avec les outils actuels, par exemple en faisant:

TENANT_MAPPING = json.load(file('/etc/<logiciel>/tenants/tenant_mapping.conf'))

L'autre souci c'est que pour l'instant l'exécution d'une commande donne ça:

(authentic2-venv)bdauvergne@fenouil:~/Code/authentic2$ ./run.sh get_tenant_by_domain test.localhost
Cannot access global cache path, using in project cache directory /home/bdauvergne/Code/authentic2/authentic2/../cache

Warning: You should put 'tenant_schemas' at the end of INSTALLED_APPS like this:
INSTALLED_APPS = TENANT_APPS + SHARED_APPS + ('tenant_schemas',)
This is neccesary to overwrite built-in django management commands with their schema-aware implementations.

test_localhost

Le premier message ça ira mais le deuxième n'est pas aussi simple à faire taire.

Benjamin Dauvergne a écrit :

L'autre souci c'est que pour l'instant l'exécution d'une commande donne ça:

[...]

Le premier message ça ira mais le deuxième n'est pas aussi simple à faire taire.

J'ai fait une pull request pour ça upstream.

https://github.com/bernardopires/django-tenant-schemas/pull/171

Benjamin Dauvergne a écrit :

Thomas Noël a écrit :

Oui, un explicite /etc/<logiciel>/tenant_by_domain.json (et si ce fichier n'existe pas, c'est pas grave).

Finalement c'est un dico dans les settings nommé TENANT_MAPPING.

Je trouve ça dommage, ça va demander un "restart" de l'application alors que le reste était dynamique.

Thomas Noël a écrit :

Benjamin Dauvergne a écrit :

Thomas Noël a écrit :

Oui, un explicite /etc/<logiciel>/tenant_by_domain.json (et si ce fichier n'existe pas, c'est pas grave).

Finalement c'est un dico dans les settings nommé TENANT_MAPPING.

Je trouve ça dommage, ça va demander un "restart" de l'application alors que le reste était dynamique.

... et je viens de lire que ça a déjà été discuté, ok pour le "kill -HUP", considérant qu'en mode normal de déploiement, on ne devrait pas avoir besoin d'un TENANT_MAPPING.

Thomas Noël a écrit :

... et je viens de lire que ça a déjà été discuté, ok pour le "kill -HUP", considérant qu'en mode normal de déploiement, on ne devrait pas avoir besoin d'un TENANT_MAPPING.

Et je confirme ici que le kill -HUP ça me va très bien aussi.

Benjamin Dauvergne a écrit :

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

Une mise à jour de python-entrouvert sur un serveur avec des applications utilisant les tenants, ça va passer sans peine ? (on s'en fout si on n'a aucune installation utilisant des tenants pour le moment, c'est juste que je ne suis pas sûr de ça)

Et sur ça je laisse béton ?

Non c'est une mise à jour incompatible mais je peux restaurer ce que j'ai cassé volontairement:

• la classe Tenant que j'ai rendu inopérante pour ne pas se retrouver avec des modèles créés quand on n'en veut pas
• la classe EOTenantMiddleware qui chargeait des settings depuis la base
• restauré l'ancien FilesystemLoader qui n'ajoutait pas templates/ à la fin du chemin des templates pour un tenant et utilisait MULTITENANT_TEMPLATE_DIRS comme variable et pas TENANT_TEMPLATE_DIRS, mais dans ce cas il faudrait que je créé un autre classe avec un nom différent pour le nouvelle version

Et sur ça je laisse béton ?

J'avais juste fait une réponse rapide sur le point, pour dire que j'étais d'accord avec Thomas aussi.

Sur les autres points; j'écrivais :

(on s'en fout si on n'a aucune installation utilisant des tenants pour le moment, c'est juste que je ne suis pas sûr de ça)

Je maintiens ça; ça m'irait vraiment très bien de ne pas transporter de legacy. Tu sais s'il y a du multitenant quelque part dans des applications déployées en prod ? (le seul utilisateur "public" de tenant qui me vienne, c'est le poc univnautes in the cloud de Thomas, sur lequel on ne doit pas bloquer).

Non mais je vais corriger get_tenant_by_domain, pour l'instant il ne retourne la bonne valeur que si le répertoire du tenant existe déjà.

Une fois cela ok, les quatre lignes que je donne pour le déploiement depuis un agent hobo sont donc correctes ? Si c'est le cas ça me va bien.

Non il y a toujours le souci des lignes en sortie gênantes. Actuellement il faut faire comme ça (ajout de | tail -n1):

Ah oui, ça dans leur init.py, c'est moche :

if settings.INSTALLED_APPS[-1] != 'tenant_schemas':
    print recommended_config

Tu tentes une pull request ajoutant l'envoi vers sys.stderr ?

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

Non il y a toujours le souci des lignes en sortie gênantes. Actuellement il faut faire comme ça (ajout de | tail -n1):

Ah oui, ça dans leur init.py, c'est moche :

[...]

Tu tentes une pull request ajoutant l'envoi vers sys.stderr ?

Déjà fait #5106#note-28 (solution un peu différente).

Benjamin Dauvergne a écrit :

Non rien à ma connaissance et je n'ai pas connaissance non plus d'un poc univnautes in the cloud multitnenant pour l'instant.

Je confirme que ça a existé, mais on peut vraiment faire comme si ça n'existait pas (c'était un bidule sur https://univnautes-idp.dev.entrouvert.org/). Il n'y a pas de code historique à maintenir dans python-entrouvert au sujet des tenants (youpi).

Benjamin Dauvergne a écrit :

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

Non il y a toujours le souci des lignes en sortie gênantes. Actuellement il faut faire comme ça (ajout de | tail -n1):

Ah oui, ça dans leur init.py, c'est moche :

[...]

Tu tentes une pull request ajoutant l'envoi vers sys.stderr ?

Déjà fait #5106#note-28 (solution un peu différente).

Mon patch n'est pas évident j'ai du expliquer un peu https://github.com/bernardopires/django-tenant-schemas/pull/171#issuecomment-53238189

Benjamin Dauvergne a écrit :

Benjamin Dauvergne a écrit :

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

Ah oui, ça dans leur init.py, c'est moche :
[...]
Tu tentes une pull request ajoutant l'envoi vers sys.stderr ?

Déjà fait #5106#note-28 (solution un peu différente).

Mon patch n'est pas évident j'ai du expliquer un peu https://github.com/bernardopires/django-tenant-schemas/pull/171#issuecomment-53238189

Si ça passe pas, faudra re-tenter avec juste le >>sys.stderr et insister un peu..

Mais plus généralement, vu qu'on ne maîtrisera jamais la sortie du manage.py, on peut la formater de notre côté. Par exemple :

class Command(BaseCommand):
    help = "Create schemas"   # tiens d'ailleurs ça va pas, ça ;)

    def handle(self, *args, **options):
        for arg in args:
            print "get_tenant_by_domain %s: %s" % (arg, TenantMiddleware.hostname2schema(arg))

ce qui permet de faire un peu moins "magique" : manage.py get_tenant_by_domain mon.site | grep "get_tenant_by_domain mon.site:" | cut -f2 -d:

(enfin c'est surtout moins magique pour les vieux shellmen moi, on va dire).

ce qui permet de faire un peu moins "magique" : manage.py get_tenant_by_domain mon.site | grep "get_tenant_by_domain mon.site:" | cut -f2 -d:

Je n'aime vraiment pas :/

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

ce qui permet de faire un peu moins "magique" : manage.py get_tenant_by_domain mon.site | grep "get_tenant_by_domain mon.site:" | cut -f2 -d:

Je n'aime vraiment pas :/

Pour moi le mieux pour l'instant vu qu'on package django-tenant-schemas à la main c'est de corriger à ce moment.

C'était quand même utile d'avoir la possibilité de placer du Python par derrière, pour avoir la main sur des trucs qu'on n'arriverait pas à exprimer en json, ou pour poser une spécificité qu'on ne maitriserait pas via le déploiement.