Produits Entr'ouvert » Hobo

Il y a déjà du code différent pour appeler le delete_tenant de w.c.s.; plutôt qu'ajouter du code ici, j'appellerais la commande backup qui existe déjà pour produire un .tar.gz, qu'il est évidemment possible d'étendre pour inclure également un dump sql dedans.

Mais je ne mêlerais pas archivage et suppression dans la même commande.

Pardon, je parlais de w.c.s., là. (mais pour suivre ce que fait w.c.s. avoir une commande d'archivage différente de la commande de suppression, ça me semble avoir du sens).

Ok donc création d'une commande d'archivage (dump et zip des répertoires dans une archive tar.gz).

Jean-Baptiste Jaillet a écrit :

Hum j'ai fait l'archivage (je ne mets rien pour l'instant en l'absence de test et dans un souci de préserver votre patience).
J'ai une question pour la parti pg_dump :
Déjà pour être sûr pour les settings :
[...]
Et la (j'ai un doute sur le DB['USER'] mit en commentaire du ticket #16302 de benj) j'ai accès à DATABASES['default']['HOST'] et ['USER'], mais aussi password si je dis pas de bêtise.
Du coup pourquoi passé par la variable d'environnement PGPASSWD?

Parce que pg_dump ne sait pas recevoir de mot de passe sur la ligne de commande, voir man pg_dump:

       -w
       --no-password
           Never issue a password prompt. If the server requires password authentication and a password is not available by other means such as a .pgpass file, the
           connection attempt will fail. This option can be useful in batch jobs and scripts where no user is present to enter a password.

Si on passe par PGPASSWD, est-ce que du coup elle est set au moment de l'appel comme on est connecté avec le tenant?

Pas compris, parle français.

Et ensuite l'appel serait un :
[...]

Et j'ajoute le dump dans le repertoire de backup qui contient aussi le tarball

Oui.

merci Benj mais du coup j'ai mis à jour la réponse hier soir. (ou j'ai changé le -w et je set bien PGPASSWORD). J'ai mon souci avec les tests.

Ça ne va pas, il y a déjà une classe de base pour faire des commandes destinées à agir sur un tenant, pas la peine de refaire cela (faut lire du code avant d'en écrire :) ) dans hobo.multitenant.management.commands tu as InteractiveTenantOption et TenantWrappedCommand, il vaut mieux partir de InteractiveTenantOption, TenantWrappedCommand servant plutôt à ajouter le support des tenants à une commande existante, comme ceci:

from django.db import connection

class MaCommande(InteractiveTenantOption, BaseCommand):

    ....

    def handle(*args, **options):
        tenant = self.get_tenant_from_options_or_interactive(**options)
        connection.set_tenant(tenant)

        <faire ce qu'il y a à faire>

Jean-Baptiste Jaillet a écrit :

Bon, j'ai modifié tout ça:

• rajouté le port (connection à distance, le port peut être différent)

Ok, extrait le schéma de connection.tenant.schema_name en utilisant le code plus haut.

Ça je suis sûr que ça ne marche pas:

            if db_pwd:
                subprocess.check_call(['PGPASSWORD="%s" && export PGPASSWORD' % db_pwd])

L'environnement n'est pas maintenue entre plusieurs appels à check_call(), lis un bouquin Unix sur fork/etc.. Le plus simple c'est de faire:

import os
os.environ['PGPASSWORD'] = db_pwd

Aussi pour formatter une date au format ISO, timezone.now().date().isoformat() c'est plus sûr, on finit toujours par se tromper avec les chaînes de formatage.

• le nom de la db (et un truc un peu cheap en cas d'user 'root' qui se connecte et du coup ne trouve pas le schema puisqu'il n'est pas connecté à la base de l'appli. Donc je récupère la base par le nom du service. CF: je demandais conseil plus haut, sûrement un problème de settings des tests multitenant mais j'ai gratté tout ce que je pouvais et j'ai trouvé que ça pour passer les tests. Sinon en temps normal, on récupère le nom de la db (ce qui n'est pas obligatoire) depuis l'entrée DATABASE['default']['NAME']

Incompréhensible, et surtout je ne vois aucune référence à root dans le patch donc je ne sais pas de quoi tu parles, mais oui tout doit venir de DATABASE['default'] sauf le nom du schéma qui vient de connection.tenant.schema_name, c'est tout il n'y a rien d'autre à dire. S'il faut les droits de l'utilisateur du service on les aura car les commande *-manage doivent être lancé avec l'utilisateur du service.

• l'option -w, en relisant bien la doc dans mon patch plus haut j'avais fait ça n'importe comment. Donc je précise -w pour éviter le prompt, et en cas de pwd pour se connecter (ce qui sera le cas chez sictiam puisque connection à distance et chaque db a un pwd), je set PGPASSWORD puis une fois le dump fait j'unset la variable (on ne sait jamais, je me dis que si on la laisse traîner ça peut causer des soucis ensuite).

L'environnement des processus ne fonctionne pas comme ça, il est hérité du processus père et copié dans le processus fils lors d'un fork, ce n'est pas un objet global, il est local au processus donc tes check_call() ne servent à rien.

Du coup les tests passent, mais c'est un peu de la 'triche' :

Les tests passent parce qu'il n'y a pas de mot de passe. Faudra faire des tests à la main.

[...]
Donc encore une fois, je ne sais pas comment sont paramétrés les tests mais depuis le tox il me manque un truc ([...] depuis la racine de l'app django)

C'est un peu tard donc j'espère que ce que j'écris est compréhensible.

Non. :)

• quand tu crées un fichier depuis un programme il faut le faire dans un endroit temporaire avec le module tempfile et si tout se passe bien le déplacer à sa destination. En cas d'erreur il faut nettoyer (supprimer les fichiers temporaires, supprimer récursivement le répertoire de destination qui vient d'être créé, etc..). Le plus simple c'est peut-être de créer un répertoire temporaire de destination avec tempfile.mkdtemp puis si il y a une exception dans le chemin non-passant (except: quoi) le supprimer avec shutil.rmtree et dans le chemin passant le déplacer à sa position finale (dans --destination donc) avec shutil.move.
• pour ton tarball, te prends pas la tête avec tarfile il y a toutes les chances que tu te plantes, utilise tar tout bêtement,
• ajoute le nom de l'application dans le nom des fichiers de dump <settings.PROJECT_NAME>__<schema_name>__<date>.sql, on pourra imaginer un jour pouvoir restaurer ce genre de backups simplement depuis les noms,
• étudie la possibilité d'avoir pour destination un zip plutôt qu'un répertoire (et là pareil, on génère d'abord sql et tgz dans des fichiers temporaires et quand tout va bien on ajoute au zip, pour que ce soit transactionnel, qu'on se retrouve pas avec un zip tout seul sans son tarball parce qu'un truc a foiré au milieu)

Jean-Baptiste Jaillet a écrit :

pour ton tarball, te prends pas la tête avec tarfile il y a toutes les chances que tu te plantes, utilise tar tout bêtement

j'ai cherché j'ai trouvé que tarfile en lib (ou alors j'ai pas compris la remarque, si c'est plus sur le mode d'ouverture w:gz) et je me suis inspiré de ce qui est dans la commande backup de wcs.

Pour tar je parle de la commande shell, Popen(['tar', 'cvzf', tar_pat, '/var/lib/<app>/<tenant>/'])@.

• PROJECT_NAME n'est pas mis dans le nom du fichier zip uniquement dans le nom du dump mais pas dans le nom du tarball, c'est quoi ta logique là ? Tu ne peux pas tout nommer pareil ? Le dump sql l'extension c'est ".sql" pas ".dump" qui n'est pas un format.
• faut chopper toutes les erreurs, sur mkdtemp et sur rmtree notamment (en gros tout ce qui fait des IOs il faut mettre un try/except autour)

• "dump failed" -> "backup of postgresql schema %s of database %s failed"
• "targz creation failed" -> "backup of /var/lib/etc./tenant/etc../ failed"

Pour faciliter le traitement des erreurs je te conseillerai de découper ta fonction en fonctions plus petites:

def handle(...):
    temp_dir = ...
    try:
        mkdtemp(temp_dir)
    except IOError as e:
        etc..

    try:
        pg_backup_path = self.backup_postgresql(temp_dir)
        tenant_dir_backup_path = self.backup_tenant_dir(temp_dir)
        self.create_zip(temp_dir, pg_backup_path, tenant_dir_backup_path)
    finally:
        try:
            shutil.rmtree(temp_dir)
        except IOError as e:
            print 'cannot remove %s' % temp_dir

Ça t'évite d'écrire shutil.rmtree partout et on comprend mieux les étapes (je crois qu'on appelle ça de la programmation structurée :) ).

Aussi il faudrait utiliser deux underscores pour séparer application, hostname et date, qu'on puisse facilement faire l'opération inverse.

Termine-t-on ce ticket ? (je veux bien le reprendre si on juge ça utile)

Personnellement ça ne m'a jamais manqué.