entrouvert
/
passerelle
15
0
Fork 0
Code Issues Pull Requests 15 Releases Activity

caldav: add EGW SQL log cleaning (#88974) #510

Merged
yweber merged 1 commits from wip/88974-handle-random-sql-log-in-reply into main 2024-04-03 13:46:19 +02:00
Conversation 0 Commits 1 Files Changed 2 +78
yweber commented 2024-04-02 17:08:56 +02:00
Owner
Copy Link
No description provided.
yweber added 2 commits 2024-04-02 17:08:57 +02:00
gitea/passerelle/pipeline/head This commit looks good Details
ad54401802 caldav: fix SEQUENCE value when creating an event (#88977)
gitea/passerelle/pipeline/head There was a failure building this commit Details
57fea6a8bd caldav: add EGW SQL log cleaning (#88974)
yweber force-pushed wip/88974-handle-random-sql-log-in-reply from 57fea6a8bd to 191d2d3045 2024-04-02 17:20:52 +02:00 Compare
yweber changed title from WIP: wip/88974-handle-random-sql-log-in-reply to wip/88974-handle-random-sql-log-in-reply 2024-04-02 17:24:36 +02:00
vdeniaud requested changes 2024-04-02 18:01:20 +02:00
Dismissed
passerelle/apps/caldav/models.py Outdated
@ -114,6 +114,22 @@ EVENT_SCHEMA = {
}
def clean_EGW_response(req, *args, **kwargs):
vdeniaud commented 2024-04-02 17:33:40 +02:00
Owner
Copy Link

Function names should be lowercase

https://peps.python.org/pep-0008/#function-and-variable-names :)

> Function names should be lowercase https://peps.python.org/pep-0008/#function-and-variable-names :)
yweber marked this conversation as resolved
passerelle/apps/caldav/models.py Outdated
@ -117,0 +122,4 @@
- startswith "==> SQL =>"
- endswith "<br>"
'''
req._content = b'\n'.join(
vdeniaud commented 2024-04-02 17:55:39 +02:00
Owner
Copy Link

C'est une réponse qu'on manipule ici, autant l'appeler response :)

C'est une réponse qu'on manipule ici, autant l'appeler `response` :)
yweber marked this conversation as resolved
passerelle/apps/caldav/models.py Outdated
@ -117,0 +123,4 @@
- endswith "<br>"
'''
req._content = b'\n'.join(
spl
vdeniaud commented 2024-04-02 17:46:18 +02:00
Owner
Copy Link

Cette variable pourrait s'appeler line, toujours mieux d'avoir des vrais mots !

Cette variable pourrait s'appeler `line`, toujours mieux d'avoir des vrais mots !
yweber marked this conversation as resolved
passerelle/apps/caldav/models.py Outdated
@ -117,0 +124,4 @@
'''
req._content = b'\n'.join(
spl
for spl in req.text.encode().split(b'\n')
vdeniaud commented 2024-04-02 17:52:09 +02:00
Owner
Copy Link

Sûrement plus safe de choper le binaire direct avec req.content

Sûrement plus safe de choper le binaire direct avec `req.content`
yweber commented 2024-04-02 18:07:42 +02:00
Author
Owner
Copy Link

Bien vu ! J'avais commencé par testé ça, mais content était à False : sûrement un cas particulier durant mes tests !

Bien vu ! J'avais commencé par testé ça, mais `content` était à `False` : sûrement un cas particulier durant mes tests !
yweber marked this conversation as resolved
passerelle/apps/caldav/models.py Outdated
@ -117,0 +125,4 @@
req._content = b'\n'.join(
spl
for spl in req.text.encode().split(b'\n')
if not spl.startswith(b'==> SQL =>') or not spl.endswith(b'<br>')
vdeniaud commented 2024-04-02 17:48:15 +02:00
Owner
Copy Link

Détail mais je pensais que tu m'avais dit que le SQL était toujours en première ligne avant le XML, ça aurait permis de ne pas faire référence à un format de log qui a plus de chance de changer que le format caldav ?

Détail mais je pensais que tu m'avais dit que le SQL était toujours en première ligne avant le XML, ça aurait permis de ne pas faire référence à un format de log qui a plus de chance de changer que le format caldav ?
yweber commented 2024-04-02 18:23:38 +02:00
Author
Owner
Copy Link

Navré si j'ai pas été clair.

  • dans les deux cas que j'ai dans les logs la ligne de SQL était au début (mais 2 c'est peu)
  • j'ai déja vu du SQL en plein milieu d'une réponse XML quand j'attaquais EGW avec cURL (j'imagine pour un PROPFIND ?)

Donc, en fait, je ne sais pas si le message de log est vraiment toujours en première ligne :/

Et oui, totalement d'accord avec toi sur le principe de matcher le format de la ligne de log. Cependant, dans notre cas, la ligne de log SQL ne changera pas sans MàJ d'EGW. Du coup je trouvais ça relativement raisonnable d'identifier le ligne de log avec son prefix ( https://github.com/ADOdb/ADOdb/blob/master/drivers/adodb-postgres64.inc.php#L574 ) et son suffix ( https://github.com/ADOdb/ADOdb/blob/master/adodb.inc.php#L916 ) qui :

  • ne devrait pas changer de ci-tôt (et on peut espérer que le jour ou elle change ça sera pour disparaître :P )
  • ne devrait pas provoquer de faux positif avec ICS (et a très peu de chance d'en provoquer avec le XML renvoyé à priori)

Mais si tu préfère je peux :

  • ne tester que la première ligne de la réponse quand on reçois de l'ICS (vérifier qu'elle commence avec [A-Z] par exemple)
  • enlever les tests pour le PROPFIND
Navré si j'ai pas été clair. - dans les deux cas que j'ai dans les logs la ligne de SQL était au début (mais 2 c'est peu) - j'ai déja vu du SQL en plein milieu d'une réponse XML quand j'attaquais EGW avec cURL (j'imagine pour un PROPFIND ?) Donc, en fait, je ne sais pas si le message de log est vraiment toujours en première ligne :/ Et oui, totalement d'accord avec toi sur le principe de matcher le format de la ligne de log. Cependant, dans notre cas, la ligne de log SQL ne changera pas sans MàJ d'EGW. Du coup je trouvais ça relativement raisonnable d'identifier le ligne de log avec son prefix ( https://github.com/ADOdb/ADOdb/blob/master/drivers/adodb-postgres64.inc.php#L574 ) et son suffix ( https://github.com/ADOdb/ADOdb/blob/master/adodb.inc.php#L916 ) qui : - ne devrait pas changer de ci-tôt (et on peut espérer que le jour ou elle change ça sera pour disparaître :P ) - ne devrait pas provoquer de faux positif avec ICS (et a très peu de chance d'en provoquer avec le XML renvoyé à priori) Mais si tu préfère je peux : - ne tester que la première ligne de la réponse quand on reçois de l'ICS (vérifier qu'elle commence avec `[A-Z]` par exemple) - enlever les tests pour le PROPFIND
vdeniaud commented 2024-04-03 10:44:54 +02:00
Owner
Copy Link

Je viens de tester avec la récup des congés depuis barbacompta, où le SQL sort presque de manière déterministe, et effectivement je reçois une réponse qui commence par :

b'<?xml version="1.0" encoding="utf-8"?>\n<D:multistatus xmlns:D="DAV:">\n==> SQL => SELECT d.adnum as num, d.adsrc as def from pg_attrdef d, pg_class c where d.adrelid=c.oid and c.relname=\'egw_cal\' order by d.adnum<br>\n <D:response xmlns:ns0="urn:uuid:c2f41010-65b3-11d1-a29f-00aa00c14882/" xmlns:ns2="urn:ietf:params:xml:ns:caldav">\n

Pour une requête qui retourne des évènements.

Donc laissons comme ça !

Je viens de tester avec la récup des congés depuis barbacompta, où le SQL sort presque de manière déterministe, et effectivement je reçois une réponse qui commence par : ``` b'<?xml version="1.0" encoding="utf-8"?>\n<D:multistatus xmlns:D="DAV:">\n==> SQL => SELECT d.adnum as num, d.adsrc as def from pg_attrdef d, pg_class c where d.adrelid=c.oid and c.relname=\'egw_cal\' order by d.adnum<br>\n <D:response xmlns:ns0="urn:uuid:c2f41010-65b3-11d1-a29f-00aa00c14882/" xmlns:ns2="urn:ietf:params:xml:ns:caldav">\n ``` Pour une requête qui retourne des évènements. Donc laissons comme ça !
yweber changed title from wip/88974-handle-random-sql-log-in-reply to caldav: add EGW SQL log cleaning (#88974) 2024-04-02 18:24:34 +02:00
yweber force-pushed wip/88974-handle-random-sql-log-in-reply from 191d2d3045 to 6741614992 2024-04-02 18:24:55 +02:00 Compare
yweber requested review from vdeniaud 2024-04-03 09:56:22 +02:00
vdeniaud approved these changes 2024-04-03 10:44:59 +02:00
yweber force-pushed wip/88974-handle-random-sql-log-in-reply from 6741614992 to 23a59e3f0f 2024-04-03 11:06:13 +02:00 Compare
yweber merged commit 23a59e3f0f into main 2024-04-03 13:46:19 +02:00
yweber deleted branch wip/88974-handle-random-sql-log-in-reply 2024-04-03 13:46:19 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
vdeniaud
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 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/passerelle#510
No description provided.
Delete Branch "wip/88974-handle-random-sql-log-in-reply"

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?