Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

edi_xml_oca: replace xmlschema with lxml to implement XML Schema validation #127

Open
gurneyalex opened this issue Dec 10, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@gurneyalex
Copy link
Member

gurneyalex commented Dec 10, 2024

Issue

the edi_xml_oca module uses xmlschema for the validation of XML documents. This is a pure Python library, and is can be really slow, especially with complex schemas, which are unfortunately numerous in the EDI world.

We propose to replace the implementation with lxml, with is libxml2 backed (or to use lxml if it is available, and fall back to xmlschema in case lxml is unavailable) to get a faster implementation.

@gurneyalex gurneyalex added the enhancement New feature or request label Dec 10, 2024
@simahawk
Copy link
Contributor

hanks for taking time to write this down!
I had this in mind since a while, indeed.
That lib sucks perf wise.
Feel free to move on w/ that.
The only drawback is that that lib has been used to load data as well (likely just in some tests). To be checked.

@simahawk
Copy link
Contributor

FTR you have an example of validation here https://github.com/odoo/odoo/blob/834e8a8789d1d451f62ae1e841845baf7d3931a4/odoo/tools/xml_utils.py#L89 but it relies on having xsd as an attachment.

@Ricardoalso
Copy link

I opted to use the xmltodict library to convert the xml_content into a dictionary, replacing the previous approach that used the to_dict method from xmlschema. Additionally, @gurneyalex discovered this Stack Overflow discussion featuring an intriguing raw Python implementation. Do you have any preferences on this choice, @simahawk?

One open question remains: how should we handle the import and usage of this new dependency to maintain backward compatibility? Any thoughts or preferences, @simahawk?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants