[16.0][IMP] edi_xml_oca: replace xmlschema by lxml #129
Conversation
|
Hi @simahawk, |
Ricardoalso
force-pushed
the
lxml
branch
3 times, most recently
from
b6a65ac to
cd9d00e
Compare
| try: | ||
| return self.schema.validate(xml_content) | ||
| except self._validate_swallable_exceptions() as err: | ||
| validate_xml_from_attachment(self.env, xml_content, xsd_name) |
There was a problem hiding this comment.
Are you sure this method is doing what we want? Have you checked it's implementation?
There was a problem hiding this comment.
This public function validates the XML content with an XSD attachment; making a call to the private function _check_with_xsd that you pointed here. Actually validate_xml_from_attachment acts as a wrapper to validate XML content against an XSD schema while handling exceptions gracefully.
The method implementation can be seen here
Unless I'm missing something, yes I think the function does what we want 🤔
There was a problem hiding this comment.
have you read the docstring of that method?
If the XSD attachment cannot be found in database, skip validation without raising.
We do not have any attachment here. IMO we can use _check_with_xsd by passing directrly the stream of the XSD.
Then if we want to support attachments too, ok but this is not the case today.
There was a problem hiding this comment.
Indeed I missed the point that it was not being stored. I refactored the code and added a new test to check if XML validation was done correctly
Ricardoalso
force-pushed
the
lxml
branch
5 times, most recently
from
fe4b0bc to
aee5cb8
Compare
Addressing the issue #127