-
Notifications
You must be signed in to change notification settings - Fork 40
aasx.py: Add failsafe for writing and reading AASX files #378
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
Conversation
s-heppner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I see two things that still need some discussion:
- Do we really need the new class
FailsafeConfigurable? It is a very shallow class with only one boolean parameter. I get that it standardizes the interface of how to interact with the failsafe configuration, but on the other hand, it adds additional complexity to each of the classes that inherit from it, for what could also be a simple attribute that "by chance" (our design) is the same across all of them. - We could think about in some cases to return a
logger.errorinstead oflogger.warning, This still doesn't cause an exception, but for example when no AASX could be found, this could be seen as a more severe problem than just awarning.
Let's talk about this in our next meeting.
|
I adapted one |
s-heppner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you go through the adapter.aasx one more time, adapting all the different string formatting syntaxes to only use "f-strings" (e.g. f"Submodel {submodel} could not be found or similar)?
When this was first written, f-strings did not exist yet and imo it's a bit messy that different syntax styles are used within one project, so this is a good opportunity to start cleaning this up, at least for aasx now.
s-heppner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you very much!
…yx#378) Previously, the `failsafe` parameter, allowing to skip errors and only log them as warnings instead, only existed for JSON and XML (de-)serialization. This PR extends the feature to also be included when reading and writing AASX files. For this, we add the `failsafe` boolean attribute to the `AASXReader` and `AASXWriter` classes in `adapter.aasx`. If `failsafe` is `True`, the document is parsed in a failsafe way: Missing attributes and elements are logged instead of causing exceptions. Defect objects are skipped. The default value is `False`, keeping the existing behavior. Furthermore, we do a little code-style clean up on string-formatting in the `adapter.aasx` module, updating all occurrences from (historically) different syntaxes to the more modern f-string syntax. Fixes eclipse-basyx#228
Previously, the
failsafeparameter, allowing to skip errors and only log them as warnings instead, only existed for JSON and XML (de-)serialization.This PR extends the feature to also be included when reading and writing AASX files.
For this, we add the
failsafeboolean attribute to theAASXReaderandAASXWriterclasses inadapter.aasx.If
failsafeisTrue, the document is parsed in a failsafe way: Missing attributes and elements are logged instead of causing exceptions. Defect objects are skipped. The default value isFalse, keeping the existing behavior.Furthermore, we do a little code-style clean up on string-formatting in the
adapter.aasxmodule, updating all occurrences from (historically) different syntaxes to the more modern f-string syntax.Fixes #228