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

switch to lxml to have a complete XML parser (#78281) #362

Merged
pducroquet merged 1 commits from wip/78281-odt-namespaces into main 2023-06-14 13:07:42 +02:00
Conversation 6 Commits 1 Files Changed 50 +63 -113
pducroquet commented 2023-06-09 07:04:56 +02:00
Owner
Copy Link
No description provided.
pducroquet force-pushed wip/78281-odt-namespaces from 841d3dd73b to ab29b5a81a 2023-06-09 08:34:27 +02:00 Compare
pducroquet force-pushed wip/78281-odt-namespaces from ab29b5a81a to 67707c1cbd 2023-06-09 08:51:11 +02:00 Compare
pducroquet force-pushed wip/78281-odt-namespaces from e8603bbb91 to 21237ec92f 2023-06-09 11:13:43 +02:00 Compare
pducroquet changed title from WIP: export_to_model: switch to lxml to have a complete XML parser (#78281) to export_to_model: switch to lxml to have a complete XML parser (#78281) 2023-06-11 09:04:57 +02:00
pducroquet force-pushed wip/78281-odt-namespaces from a9d6313260 to b3b70236f7 2023-06-11 09:12:38 +02:00 Compare
fpeters requested changes 2023-06-12 10:27:23 +02:00
fpeters left a comment
Owner
Copy Link

(et une fois ok tout squasher).

(et une fois ok tout squasher).
wcs/fields.py Outdated
@ -29,6 +29,7 @@ import xml.etree.ElementTree as ET
from django.utils.encoding import force_bytes, force_str, smart_str
from django.utils.formats import date_format as django_date_format
from django.utils.html import strip_tags, urlize
from lxml import etree as LET
fpeters commented 2023-06-11 09:19:15 +02:00
Owner
Copy Link

Ça se joue de remplacer partout l'import actuel par celui-ci, et conserver ET comme nom ? Ça évitera je pense les confusions entre les endroits ET et les endroits LET et quoi utiliser où.

Sinon, plutôt faire comme les autres modules et être explicite lors des utilisations, lxml.etree.Element, pour les différencier davantage à la lecture ?

Ça se joue de remplacer partout l'import actuel par celui-ci, et conserver ET comme nom ? Ça évitera je pense les confusions entre les endroits ET et les endroits LET et quoi utiliser où. Sinon, plutôt faire comme les autres modules et être explicite lors des utilisations, lxml.etree.Element, pour les différencier davantage à la lecture ?
tnoel approved these changes 2023-06-12 10:37:59 +02:00
Dismissed
tnoel left a comment
Owner
Copy Link

Sans savoir si c'est facile (j'en doute), si ça débogue quelque chose on pourrait avoir un test ?

Sans savoir si c'est facile (j'en doute), si ça débogue quelque chose on pourrait avoir un test ?
tnoel dismissed tnoel’s review 2023-06-12 10:40:01 +02:00
Reason:

erreur de manip

fpeters commented 2023-06-12 10:41:07 +02:00
Owner
Copy Link

Sans savoir si c'est facile (j'en doute), si ça débogue quelque chose on pourrait avoir un test ?

Le hack que j'avais introduit venait avec un test, donc ici qui retire le hack et le test continue à fonctionner ça me va.

(test_formdata_generated_document_ods_download)

> Sans savoir si c'est facile (j'en doute), si ça débogue quelque chose on pourrait avoir un test ? Le hack que j'avais introduit venait avec un test, donc ici qui retire le hack et le test continue à fonctionner ça me va. (test_formdata_generated_document_ods_download)
pducroquet changed title from export_to_model: switch to lxml to have a complete XML parser (#78281) to WIP: export_to_model: switch to lxml to have a complete XML parser (#78281) 2023-06-12 12:42:04 +02:00
pducroquet force-pushed wip/78281-odt-namespaces from fe73bdf9a2 to 197e0b1a24 2023-06-14 10:58:57 +02:00 Compare
pducroquet changed title from WIP: export_to_model: switch to lxml to have a complete XML parser (#78281) to switch to lxml to have a complete XML parser (#78281) 2023-06-14 11:24:10 +02:00
pducroquet commented 2023-06-14 11:25:46 +02:00
Author
Owner
Copy Link

Alors j'ai tenté le remplacement par lxml dans les fichiers concernés plutôt qu'importer lxml et xml.etree... Le patch a changé de volume puisque de fil en aiguille ça a fait remplacer tous les usages de xml.etree par lxml.
Si c'est souhaitable/acceptable, le patch est là.
Sinon je reprends sur l'état précédent et je change juste les imports pour que l'utilisation de lxml soit explicite.

Alors j'ai tenté le remplacement par lxml dans les fichiers concernés plutôt qu'importer lxml et xml.etree... Le patch a changé de volume puisque de fil en aiguille ça a fait remplacer tous les usages de xml.etree par lxml. Si c'est souhaitable/acceptable, le patch est là. Sinon je reprends sur l'état précédent et je change juste les imports pour que l'utilisation de lxml soit explicite.
pducroquet requested review from fpeters 2023-06-14 11:25:56 +02:00
fpeters commented 2023-06-14 11:55:51 +02:00
Owner
Copy Link

Tu peux commenter les zones qui ne sont pas juste le changement de la ligne d'import ?

Tu peux commenter les zones qui ne sont pas juste le changement de la ligne d'import ?
pducroquet reviewed 2023-06-14 12:03:24 +02:00
tests/test_formdef_import.py Outdated
@ -111,1 +109,3 @@
assert '<display_locations />' in str(ET.tostring(f3))
assert '<display_locations/>' in str(ET.tostring(f1))
assert '<display_locations/>' in str(ET.tostring(f2))
assert '<display_locations/>' in str(ET.tostring(f3))
pducroquet commented 2023-06-14 12:03:24 +02:00
Author
Owner
Copy Link

lxml "compresse" les tags vides.

lxml "compresse" les tags vides.
pducroquet reviewed 2023-06-14 12:05:17 +02:00
wcs/backoffice/i18n.py Outdated
@ -347,3 +347,3 @@
elif self.file_format == 'xliff':
misc.indent_xml(root)
output.write(ET.tostring(root, 'utf-8'))
output.write(ET.tostring(root, encoding='utf-8'))
pducroquet commented 2023-06-14 12:05:17 +02:00
Author
Owner
Copy Link

Différence de signature lxml vs etree:
lxml: element_or_tree, *, encoding=, ...
etree: element, encoding, method, *, ...

Différence de signature lxml vs etree: lxml: element_or_tree, *, encoding=, ... etree: element, encoding, method, *, ...
pducroquet reviewed 2023-06-14 12:05:40 +02:00
wcs/qommon/ods.py Outdated
@ -150,3 +150,3 @@
def get_styles(self):
return ET.tostring(self.get_styles_node(), 'utf-8')
return ET.tostring(self.get_styles_node())
pducroquet commented 2023-06-14 12:05:40 +02:00
Author
Owner
Copy Link

Idem, différence de signature

Idem, différence de signature
pducroquet reviewed 2023-06-14 12:06:50 +02:00
wcs/snapshots.py Outdated
@ -30,4 +30,2 @@
def indent(tree, space=" ", level=0):
# backport from Lib/xml/etree/ElementTree.py python 3.9
pducroquet commented 2023-06-14 12:06:50 +02:00
Author
Owner
Copy Link

backport de python 3.9 => j'ai supprimé, lxml a la fonction requise. Accessoirement, maintenant qu'on est en bullseye minimum, on pourrait la supprimer dans tous les cas.

backport de python 3.9 => j'ai supprimé, lxml a la fonction requise. Accessoirement, maintenant qu'on est en bullseye minimum, on pourrait la supprimer dans tous les cas.
pducroquet reviewed 2023-06-14 12:07:11 +02:00
wcs/wf/export_to_model.py Outdated
@ -156,4 +147,0 @@
b':document-content ',
b':document-content xmlns:of="urn:oasis:names:tc:opendocument:xmlns:of:1.2" ',
1,
)
pducroquet commented 2023-06-14 12:07:11 +02:00
Author
Owner
Copy Link

Suppression de l'ancien hack.

Suppression de l'ancien hack.
pducroquet reviewed 2023-06-14 12:08:22 +02:00
tests/form_pages/test_formdata.py Outdated
@ -60,3 +60,3 @@
try:
# >= python 3.8: tostring preserves attribute order; use canonicalize to sort them
t1, t2 = ET.canonicalize(t1), ET.canonicalize(t2)
t1, t2 = ET.canonicalize(t1.decode("utf-8")), ET.canonicalize(t2.decode("utf-8"))
pducroquet commented 2023-06-14 12:08:22 +02:00
Author
Owner
Copy Link

Différence de signature sur canonicalize, il faut des str et non des bytes.

Différence de signature sur canonicalize, il faut des str et non des bytes.
pducroquet commented 2023-06-14 12:08:57 +02:00
Author
Owner
Copy Link

Tu peux commenter les zones qui ne sont pas juste le changement de la ligne d'import ?

C'est fait.
Je n'ai pas pu commenter pour le fichier .odt changé : j'ai du remettre tous les namespaces XML dans le fichier généré.

> Tu peux commenter les zones qui ne sont pas juste le changement de la ligne d'import ? C'est fait. Je n'ai pas pu commenter pour le fichier .odt changé : j'ai du remettre tous les namespaces XML dans le fichier généré.
fpeters approved these changes 2023-06-14 12:17:07 +02:00
fpeters left a comment
Owner
Copy Link

top; tu peux tout squasher dans un commit "general: switch xml parsing to use lxml (#78281)" ?

top; tu peux tout squasher dans un commit "general: switch xml parsing to use lxml (#78281)" ?
pducroquet force-pushed wip/78281-odt-namespaces from 197e0b1a24 to d3b3eba5ae 2023-06-14 12:24:44 +02:00 Compare
pducroquet commented 2023-06-14 12:26:05 +02:00
Author
Owner
Copy Link

squashé, par acquis de conscience je laisse jenkins tourner une dernière fois et je merge derrière ?

squashé, par acquis de conscience je laisse jenkins tourner une dernière fois et je merge derrière ?
fpeters commented 2023-06-14 13:00:28 +02:00
Owner
Copy Link

(et voilà jenkins a tourné).

(et voilà jenkins a tourné).
pducroquet merged commit d3b3eba5ae into main 2023-06-14 13:07:42 +02:00
pducroquet deleted branch wip/78281-odt-namespaces 2023-06-14 13:07:42 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
fpeters
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#362
No description provided.
Delete Branch "wip/78281-odt-namespaces"

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?