entrouvert/wcs
entrouvert
/
wcs
14
1
Fork 0
Code Issues Pull Requests 34 Releases Activity

sql: allow deploying/running in a specific postgresql schema (#50829) #906

Open
fpeters wants to merge 2 commits from wip/50829-sql-schema into main
pull from: wip/50829-sql-schema
merge into: entrouvert:main
Conversation 1 Commits 2 Files Changed 7 +306 -138
fpeters commented 2023-12-09 11:54:52 +01:00
Owner
Copy Link
No description provided.
fpeters added 1 commit 2023-12-09 11:54:53 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
4d73b4426d sql: allow deploying/running in a specific postgresql schema (#50829)
fpeters force-pushed wip/50829-sql-schema from 4d73b4426d to 335f86e2ba 2023-12-09 12:17:58 +01:00 Compare
fpeters force-pushed wip/50829-sql-schema from 335f86e2ba to 63db177bc7 2023-12-09 12:19:57 +01:00 Compare
fpeters force-pushed wip/50829-sql-schema from 63db177bc7 to 2abca6d8e6 2023-12-09 12:44:38 +01:00 Compare
fpeters force-pushed wip/50829-sql-schema from 2abca6d8e6 to b4ab47df62 2023-12-09 14:29:37 +01:00 Compare
fpeters changed title from WIP: sql: allow deploying/running in a specific postgresql schema (#50829) to sql: allow deploying/running in a specific postgresql schema (#50829) 2023-12-09 17:46:52 +01:00
pducroquet requested changes 2023-12-13 15:32:07 +01:00
Dismissed
pducroquet left a comment
Owner
Copy Link

Sur l'idée je suis largement pour, modulo le point que j'ai signalé, et deux problèmes auxiliaires qui sortent du cadre de ce ticket (persistance des connexions PG au sein d'un worker wcs, et incompatibilité du pgbouncer avec le tenant par schemas, mais ça se change/corrige dans les deux cas).

Sur l'idée je suis largement pour, modulo le point que j'ai signalé, et deux problèmes auxiliaires qui sortent du cadre de ce ticket (persistance des connexions PG au sein d'un worker wcs, et incompatibilité du pgbouncer avec le tenant par schemas, mais ça se change/corrige dans les deux cas).
wcs/sql.py Outdated
@ -98,0 +99,4 @@
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
if self.connection.set_schema:
self.execute(f'SET search_path = {self.connection.schema}')
pducroquet commented 2023-12-13 15:30:16 +01:00
Owner
Copy Link

Si je comprends bien, cela veut dire qu'à chaque fois qu'on demande un curseur, un appel à SET search_path est fait ? Je ne pense pas qu'on veuille que ce soit aussi massif, cela augmenterait la latence de l'application en multipliant les allers-retours avec le PostgreSQL. Ne peut-on pas travailler au niveau d'un middleware wsgi comme dans django ?

Si je comprends bien, cela veut dire qu'à chaque fois qu'on demande un curseur, un appel à `SET search_path` est fait ? Je ne pense pas qu'on veuille que ce soit aussi massif, cela augmenterait la latence de l'application en multipliant les allers-retours avec le PostgreSQL. Ne peut-on pas travailler au niveau d'un middleware wsgi comme dans django ?
fpeters commented 2023-12-13 16:21:01 +01:00
Author
Owner
Copy Link

Oui à chaque fois, parce qu'en regardant ce qui était produit par authentic/combo/etc. c'est ce qu'il me semblait se passer (mais peut-être c'est ma configuration en local qui rate un truc), extrait :

2023-12-13 16:12:06 CET [359046-161] fred@authentic (authentic_fred_local_0d_be) LOG:  statement: SET search_path = authentic_fred_local_0d_be,public
2023-12-13 16:12:06 CET [359046-162] fred@authentic (authentic_fred_local_0d_be) LOG:  statement: SET application_name = authentic_fred_local_0d_be
2023-12-13 16:12:06 CET [359046-163] fred@authentic (authentic_fred_local_0d_be) LOG:  statement: SELECT "authentic2_attribute"."id", "authentic2_attribute"."label", "authentic2_attribute"."description", "authentic2_attribute"."name", "authentic2_attribute"."required", "authentic2_attribute"."asked_on_registration", "authentic2_attribute"."user_editable", "authentic2_attribute"."user_visible", "authentic2_attribute"."multiple", "authentic2_attribute"."kind", "authentic2_attribute"."disabled", "authentic2_attribute"."searchable", "authentic2_attribute"."required_on_login", "authentic2_attribute"."scopes", "authentic2_attribute"."order" FROM "authentic2_attribute" WHERE (NOT "authentic2_attribute"."disabled" AND "authentic2_attribute"."name" = 'address') LIMIT 21
2023-12-13 16:12:06 CET [359046-164] fred@authentic (authentic_fred_local_0d_be) LOG:  statement: SET search_path = authentic_fred_local_0d_be,public
2023-12-13 16:12:06 CET [359046-165] fred@authentic (authentic_fred_local_0d_be) LOG:  statement: SET application_name = authentic_fred_local_0d_be
2023-12-13 16:12:06 CET [359046-166] fred@authentic (authentic_fred_local_0d_be) LOG:  statement: SELECT "authentic2_attribute"."id", "authentic2_attribute"."label", "authentic2_attribute"."description", "authentic2_attribute"."name", "authentic2_attribute"."required", "authentic2_attribute"."asked_on_registration", "authentic2_attribute"."user_editable", "authentic2_attribute"."user_visible", "authentic2_attribute"."multiple", "authentic2_attribute"."kind", "authentic2_attribute"."disabled", "authentic2_attribute"."searchable", "authentic2_attribute"."required_on_login", "authentic2_attribute"."scopes", "authentic2_attribute"."order" FROM "authentic2_attribute" WHERE (NOT "authentic2_attribute"."disabled" AND "authentic2_attribute"."name" = 'zipcode') LIMIT 21
2023-12-13 16:12:06 CET [359046-167] fred@authentic (authentic_fred_local_0d_be) LOG:  statement: SET search_path = authentic_fred_local_0d_be,public
2023-12-13 16:12:06 CET [359046-168] fred@authentic (authentic_fred_local_0d_be) LOG:  statement: SET application_name = authentic_fred_local_0d_be
2023-12-13 16:12:06 CET [359046-169] fred@authentic (authentic_fred_local_0d_be) LOG:  statement: SELECT "authentic2_attribute"."id", "authentic2_attribute"."label", "authentic2_attribute"."description", "authentic2_attribute"."name", "authentic2_attribute"."required", "authentic2_attribute"."asked_on_registration", "authentic2_attribute"."user_editable", "authentic2_attribute"."user_visible", "authentic2_attribute"."multiple", "authentic2_attribute"."kind", "authentic2_attribute"."disabled", "authentic2_attribute"."searchable", "authentic2_attribute"."required_on_login", "authentic2_attribute"."scopes", "authentic2_attribute"."order" FROM "authentic2_attribute" WHERE (NOT "authentic2_attrib
ute"."disabled" AND "authentic2_attribute"."name" = 'city') LIMIT 21

De là j'imaginais les instructions "SET ..." gratuites (surtout que là il y en a chaque fois 2), que c'était une sorte de changement de "mode opératoire" pour les vraies instructions qui suivaient. Si ça n'est pas le cas, et s'il n'y a pas de "truc" pour transmettre en un coup à la fois le SET et la requête, ça va être un peu répétitif mais assez facile, il suffit d'ajouter le schéma à toutes les requêtes et je peux faire les choses ainsi.

middleware wsgi

Non, on a dépassé ça quand on arrive au moment où le tenant est déterminé.

Oui à chaque fois, parce qu'en regardant ce qui était produit par authentic/combo/etc. c'est ce qu'il me semblait se passer (mais peut-être c'est ma configuration en local qui rate un truc), extrait : ``` 2023-12-13 16:12:06 CET [359046-161] fred@authentic (authentic_fred_local_0d_be) LOG: statement: SET search_path = authentic_fred_local_0d_be,public 2023-12-13 16:12:06 CET [359046-162] fred@authentic (authentic_fred_local_0d_be) LOG: statement: SET application_name = authentic_fred_local_0d_be 2023-12-13 16:12:06 CET [359046-163] fred@authentic (authentic_fred_local_0d_be) LOG: statement: SELECT "authentic2_attribute"."id", "authentic2_attribute"."label", "authentic2_attribute"."description", "authentic2_attribute"."name", "authentic2_attribute"."required", "authentic2_attribute"."asked_on_registration", "authentic2_attribute"."user_editable", "authentic2_attribute"."user_visible", "authentic2_attribute"."multiple", "authentic2_attribute"."kind", "authentic2_attribute"."disabled", "authentic2_attribute"."searchable", "authentic2_attribute"."required_on_login", "authentic2_attribute"."scopes", "authentic2_attribute"."order" FROM "authentic2_attribute" WHERE (NOT "authentic2_attribute"."disabled" AND "authentic2_attribute"."name" = 'address') LIMIT 21 2023-12-13 16:12:06 CET [359046-164] fred@authentic (authentic_fred_local_0d_be) LOG: statement: SET search_path = authentic_fred_local_0d_be,public 2023-12-13 16:12:06 CET [359046-165] fred@authentic (authentic_fred_local_0d_be) LOG: statement: SET application_name = authentic_fred_local_0d_be 2023-12-13 16:12:06 CET [359046-166] fred@authentic (authentic_fred_local_0d_be) LOG: statement: SELECT "authentic2_attribute"."id", "authentic2_attribute"."label", "authentic2_attribute"."description", "authentic2_attribute"."name", "authentic2_attribute"."required", "authentic2_attribute"."asked_on_registration", "authentic2_attribute"."user_editable", "authentic2_attribute"."user_visible", "authentic2_attribute"."multiple", "authentic2_attribute"."kind", "authentic2_attribute"."disabled", "authentic2_attribute"."searchable", "authentic2_attribute"."required_on_login", "authentic2_attribute"."scopes", "authentic2_attribute"."order" FROM "authentic2_attribute" WHERE (NOT "authentic2_attribute"."disabled" AND "authentic2_attribute"."name" = 'zipcode') LIMIT 21 2023-12-13 16:12:06 CET [359046-167] fred@authentic (authentic_fred_local_0d_be) LOG: statement: SET search_path = authentic_fred_local_0d_be,public 2023-12-13 16:12:06 CET [359046-168] fred@authentic (authentic_fred_local_0d_be) LOG: statement: SET application_name = authentic_fred_local_0d_be 2023-12-13 16:12:06 CET [359046-169] fred@authentic (authentic_fred_local_0d_be) LOG: statement: SELECT "authentic2_attribute"."id", "authentic2_attribute"."label", "authentic2_attribute"."description", "authentic2_attribute"."name", "authentic2_attribute"."required", "authentic2_attribute"."asked_on_registration", "authentic2_attribute"."user_editable", "authentic2_attribute"."user_visible", "authentic2_attribute"."multiple", "authentic2_attribute"."kind", "authentic2_attribute"."disabled", "authentic2_attribute"."searchable", "authentic2_attribute"."required_on_login", "authentic2_attribute"."scopes", "authentic2_attribute"."order" FROM "authentic2_attribute" WHERE (NOT "authentic2_attrib ute"."disabled" AND "authentic2_attribute"."name" = 'city') LIMIT 21 ``` De là j'imaginais les instructions "SET ..." gratuites (surtout que là il y en a chaque fois 2), que c'était une sorte de changement de "mode opératoire" pour les vraies instructions qui suivaient. Si ça n'est pas le cas, et s'il n'y a pas de "truc" pour transmettre en un coup à la fois le SET et la requête, ça va être un peu répétitif mais assez facile, il suffit d'ajouter le schéma à toutes les requêtes et je peux faire les choses ainsi. > middleware wsgi Non, on a dépassé ça quand on arrive au moment où le tenant est déterminé.
bdauvergne commented 2023-12-13 16:33:55 +01:00
Owner
Copy Link

On a sur le SaaS une configuration particulière pour ne pas appeler set search_path= à chaque curseur:

bdauvergne@front1.test:/etc$ sudo grep -Ri TENANT_LIMIT_SET_C
welco/settings.d/connection.py:TENANT_LIMIT_SET_CALLS = True
authentic2-multitenant/settings.d/connection.py:TENANT_LIMIT_SET_CALLS = True

au niveau de django-tenant-schemas ça pose un booléen sur l'objet connection dès que le schéma est défini pour ne pas répéter mais on devrait bouger ça dans debian_common_config.py.

On a sur le SaaS une configuration particulière pour ne pas appeler set search_path= à chaque curseur: ``` bdauvergne@front1.test:/etc$ sudo grep -Ri TENANT_LIMIT_SET_C welco/settings.d/connection.py:TENANT_LIMIT_SET_CALLS = True authentic2-multitenant/settings.d/connection.py:TENANT_LIMIT_SET_CALLS = True ``` au niveau de django-tenant-schemas ça pose un booléen sur l'objet connection dès que le schéma est défini pour ne pas répéter mais on devrait bouger ça dans debian_common_config.py.
bdauvergne marked this conversation as resolved
fpeters force-pushed wip/50829-sql-schema from b4ab47df62 to bf457f01e6 2023-12-14 08:58:22 +01:00 Compare
fpeters force-pushed wip/50829-sql-schema from bf457f01e6 to 48ce898bbd 2023-12-14 08:58:33 +01:00 Compare
fpeters commented 2023-12-14 09:16:50 +01:00
Author
Owner
Copy Link

J'ai posé un nouveau commit (48ce898bbd) pour faire le SET une seule fois par connexion.

J'ai posé un nouveau commit (https://git.entrouvert.org/entrouvert/wcs/commit/48ce898bbdb49b004370e4940f7801e1eccdc8ab) pour faire le SET une seule fois par connexion.
fpeters dismissed pducroquet’s review 2023-12-15 17:50:37 +01:00
Reason:

(prise en compte)

Some checks failed
gitea/wcs/pipeline/head There was a failure building this commit
Details
This pull request has changes conflicting with the target branch.
  • tests/conftest.py
  • tests/test_sql.py
  • wcs/sql.py
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/wcs#906
No description provided.
Delete Branch "wip/50829-sql-schema"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?