Produits Entr'ouvert » w.c.s.

Dans #15513 il y a renommage du schéma postgresql mais on ne peut pas avoir cette approche dans w.c.s. et on n'a pas la possibilité de s'auto-renommer. On pourrait faire un équivalent au "createdb-connection-params" (cf #15573) pour se connecter à la db postgres mais c'est un peu obscur; on peut aussi simplement passer sur toutes les tables et les renommer. (et pour le force-drop les supprimer). Ça demande cependant quand même aussi une modification à check_hobos.py pour qu'il considère également comme nouvelle base de données une base qui existerait mais n'aurait pas de table wcs_meta. Pour le renommage du répertoire du tenant, faire comme #15513 est ok.

À tester mais il me semble que la base "postgres" est généralement présente et toujours accessible à un utilisateur authentifié, donc on peut se connecter avec l'utilisateur en cours mais à la base "postgres" et ça devrait marcher.

En effet benj je viens de tester donc je peux renommer depuis postgres sur une base de wcs puisque wcs-au-quotidien a les droits suffisant.

[...] puisque wcs-au-quotidien a les droits suffisant.

Il n'y a pas de garantie là-dessus; pour que ça soit le cas il faut que l'utilisateur soit superutilisateur ou puisse créer de nouvelles bases; avec #15573 on permet justement que cette permission ne soit pas donnée à l'utilisateur (en faisant le CREATE DB selon les paramètres de 'createdb-connection-params').

wcs ne peut pas forcément créer de nouvelle base justement ? Je me souviens du paramétrage de l'utilisateur postgresql de wcs puisque quand on converti en sql wcs doit pouvoir créer une base.

avec #15573 on permet justement que cette permission ne soit pas donnée à l'utilisateur

Au-delà des mille commentaires sur la forme, s'il y a un createdb-connection-params il y a moyen de faire "CREATE DATABASE" et donc il y a aussi moyen d'en changer le nom. Si tu ne veux pas profiter de ça, tu peux juste ne pas regarder à createdb-connection-params.

Pour reprendre et développer des commentaires posés sur le salon vendredi,

• check_hobos.py
  • <soupir/> sur le code mal indenté dans check_hobos qui ne s'exécutera même pas; c'est détecté en exécutant les tests…
  • la recherche de la table wcs_meta, elle peut se faire directement dans l'SQL, table_name = 'wcs_meta'.
  • éviter les \ en fin de ligne c'est moche et souvent inutile.
  • les chaines avec de l'SQL dans le code, outre les \ à virer, il y a moyen de les rendre plus agréables à lire, espaces autour des =, alignement des mots, etc.

• delete_tenant.py
  • plutôt avoir du code en petits morceaux dans sql.py, ça facilitera l'écriture de tests.
  • parce que oui, il en faut.
  • ça facilitera la chose d'ignorer createdb-connection-params pour le moment, ça m'irait. (alternativement il pourrait s'agir de modifier le get_connection pour qu'il puisse tirer des paramètres de connexion d'ailleurs)
  • plutôt que renommer toutes les tables me vient à l'idée maintenant qu'on pourrait simplement créer un schéma postgresql et les y déplacer.
  • pris comme ça, tout le code sous la ligne 45 serait dégagé et remplacé par quelque chose comme :

for object_class in [x.data_class() for x in FormDef.select()] + [pub.user_class, pub.tracking_code_class, sql.WcsMeta]:
    object_class.remove_table(force_drop=sub_options.force_drop)

if sub_options.force_drop:
    os.rmtree(...)
else:
    os.rename(...)

Le passage par les schémas postgresql c'était une mauvaise idée dans la mesure où les autres vérifications dans information_schema.tables ne précisent pas le schéma "public"; soit il faut modifier les autres select sur information_schema.tables, soit passer par l'ancien fonctionnement (renommer les tables).

Ajouter un commentaire comme quoi avoir un "createdb-connection-params" nous indique qu'on aura ainsi les droits pour faire un DROP DATABASE. (au-dessus de createdb_cfg = pub.cfg['postgresql'].get('createdb-connection-params', {})).

Toujours cette limite postgresql sur les identifiants, pas garanti que removed_/database name/_/date/ tienne. (ex: wcs_demarches_urville_nacqueville_hague_lahague_com), et le nom de la base se trouverait ainsi tronqué et ça ferait échouer une seconde suppression.

Concernant la longueur des identifiants, si l'option prise est de renommer toutes les tables, il faut créer des noms de table de bonne taille dès le début, en tronquant la partie centrale (= l'ancien nom de table, qui aura toujours l'identifiant au début pour repérer de manière nette l'origine).

Le try/except psycopg2.Error couvre bien trop de code (et le résultat c'est que dans le except: on fait du code sur une seule itération de l'ancienne boucle sur les tables).

J'écrivais :

Concernant la longueur des identifiants, si l'option prise est de renommer toutes les tables, il faut créer des noms de table de bonne taille dès le début, en tronquant la partie centrale (= l'ancien nom de table, qui aura toujours l'identifiant au début pour repérer de manière nette l'origine).

Note qu'à dormir dessus je préférerais garder l'option du déplacement dans un schéma, en modifiant les autres SELECT sur information_schema.tables pour spécifier le schéma.

Fred je reviens sur tes retours, avec quelques questions :

Toujours cette limite postgresql sur les identifiants, pas garanti que removed_/database name/_/date/ tienne. (ex: wcs_demarches_urville_nacqueville_hague_lahague_com), et le nom de la base se trouverait ainsi tronqué et ça ferait échouer une seconde suppression.

et :

je préférerais garder l'option du déplacement dans un schéma, en modifiant les autres SELECT sur information_schema.tables pour spécifier le schéma.

Cela semble vouloir dire que l'on peut relancer la commande de suppression, mais pour moi, quelque soit l'option choisie (force-drop ou pas, utilisateur exterieur ou pas), une fois déplacer, renommer ou supprimer, l'instance devient inutilisable. C'est juste qu'on met de côté les données en cas d'erreur et pour pouvoir dumper sur une nouvelle instance.

Si on doit pouvoir garder ça, il faudrait que je modifie le publisher de l'instance non ? Puisque quand j'ai fait mes tests, j'ai remarqué qu'en changeant / supprimant la base et le pub.app_dir, les valeurs ne sont pas mises à jour, donc de toute façon, en cas de relance de la commande sur l'instance, j'aurai une erreur. Ou quelque chose m'échappe?

je préférerais garder l'option du déplacement dans un schéma, en modifiant les autres SELECT sur information_schema.tables pour spécifier le schéma.

Cela semble vouloir dire que l'on peut relancer la commande de suppression [...]

Ton patch, en l'état, déplace les tables dans un schéma particulier.

Si jamais une nouveau déploiement est fait et configuré pour utiliser la même base, ça va bizarrement foirer parce que les SELECT sur information_schema.tables vont mélanger des tables du schéma "public" en cours avec les tables archivées.

Discuté avec Thomas est-ce que la solution de remplacer les select sur les tables dans les requêtes (majoritairement dans sql.py) irait?

         cur.execute('''SELECT COUNT(*) FROM information_schema.tables
-                       WHERE table_name LIKE %s''', ('formdata\\_%s\\_%%' % new_id,))
+                       WHERE table_schema = 'public' AND
+                       table_name LIKE %s''', ('formdata\\_%s\\_%%' % new_id,))

Jean-Baptiste Jaillet a écrit :

Discuté avec Thomas est-ce que la solution de remplacer les select sur les tables dans les requêtes (majoritairement dans sql.py) irait?
[...]

Il me semble que c'est exactement ce que demandait Fred:

Note qu'à dormir dessus je préférerais garder l'option du déplacement dans un schéma, en modifiant les autres SELECT sur information_schema.tables pour spécifier le schéma.

        # if there's a createdb-connection-params, we can do a DROP DATABASE with the
        # the option --force-drop, rename it if not

the the.

-                    WHERE table_name = %s''', (table_name,))
+                   WHERE table_schema = 'public' AND
+                   WHERE table_name = %s''', (table_name,))

Code jamais exécuté, alors même que les tests passent sur ce bout :

        cur.execute('''SELECT COUNT(*) FROM information_schema.tables
                       WHERE table_schema = 'public' AND
>                      WHERE table_name = %s''', (table_name,))
E       ProgrammingError: syntax error at or near "WHERE" 
E       LINE 3:                    WHERE table_name = 'users'
E                                  ^

git am et

warning: 9 erreurs d'espace ignorées
warning: 14 lignes ont ajouté des erreurs d'espace.

(...)

+            print >> sys.stderr, 'failed to alter database %s : (%s)' % (createdb_cfg['database'],

Encore et toujours, pas d'espace devant les : en anglais.

Ça éclate si exécuté sur un wcs qui n'utilise pas postgresql :

  File ".../wcs/ctl/delete_tenant.py", line 50, in delete_tenant
    for k, v in pub.cfg['postgresql'].items():
KeyError: 'postgresql'

if 'postgresql' in pub.cfg:

Plutôt utiliser la méthode de publisher qui existe et fait ça correctement :

    def is_using_postgresql(self):
        return bool(self.has_site_option('postgresql') and self.cfg.get('postgresql', {}))

Application de  command: add a delete_tenant command (#15636)
.git/rebase-apply/patch:169: new blank line at EOF.
+
warning: 1 ligne a ajouté des erreurs d'espace.

Bim.

Vraiment, il faut [color] diff = true dans le .gitconfig, et utiliser git show.

Jean-Baptiste Jaillet a écrit :

OK j'ai ajouté ça. Mon truc de PEP 8 me conseillait d'ajouter une ligne vide à la fin de fichiers python.

Mais dans wcs/sql.py tu vires un saut de ligne à la fin du fichier.

oui puisque plus haut on me demande de l'enlever. Ou alors je n'ai pas compris :

.git/rebase-apply/patch:169: new blank line at EOF.
+
warning: 1 ligne a ajouté des erreurs d'espace.

Grâce à un bug sans trop de rapport, je me rends compte que cette approche avec createdb-connection-params dépend de conserver cette info lors du déploiement, ce qui n'est pas le cas.

je suis désolé j'ai pas bien saisi : c'est un problème du coup? on fait la commande sans la prise en compte du createdb-connection-params du coup? Ou il faut conserver les infos quelque part lors de la destruction?

je suis désolé j'ai pas bien saisi : c'est un problème du coup? on fait la commande sans la prise en compte du createdb-connection-params du coup? Ou il faut conserver les infos quelque part lors de la destruction?

C'est un problème parce que du coup toute cette partie ne sera jamais appelée. Mais il y a un patch dans #16794.

Ok donc c'est le dernier bémol pour le ack de ce patch?