fix(ua_binary): avoid recursive list codec construction for mutual da…#1950
fix(ua_binary): avoid recursive list codec construction for mutual da…#1950AndreasHeine wants to merge 11 commits intomasterfrom
Conversation
|
"Python 3.10 end-of-life (EOL) October 2026." (-_-)* |
|
Can you rebase? |
|
did i ever mention that i hate merge conflicts ^^ |
|
@oroulet could you have a look!? should fix the "recursive list codec construction" / "NotEnoughData..." and should improve the logs/trace if something similar happens again. |
asyncua/common/structures104.py
Outdated
| # Skip only if the existing class is already bound to this exact | ||
| # DataType node. Name-only checks can collide across companion specs. | ||
| if isinstance(existing_dtype, ua.NodeId) and existing_dtype == desc.NodeId: | ||
| continue |
There was a problem hiding this comment.
I am really not sure if we should do that. Everyhting is a bit wrong anyway. we should store some dict[datatype, object] somewhere in addition to using the ua namespace. I started on a MR somewhere
There was a problem hiding this comment.
to be honnest... the ExtensionObject stuff will get hardened on the way and it's likely that things will break here...
the thing which i not really like is the registration under ua module... (maybe a namespace sorted collection of datatypes would be better e.g. an explicit registry) in standardization we try to avoid similar names via prefixing... but thats not a guaranty!
however its up to you to decide! i can change it to a proper warning so at least we can see if there is a collision!?
There was a problem hiding this comment.
I am not here to make one handed decisions. If someone proposes a good solution I am all for it.
there are 2 not finished MR trying to fix that.
https://github.com/FreeOpcUa/opcua-asyncio/pull/1897/changes
and
https://github.com/FreeOpcUa/opcua-asyncio/pull/1899/changes
yes in my mind some dict[DataType, object] should do the tricks with a ua.object alias for legacy and simplicity. But making all these structure refere to that dict and not the ua type was not so easy to do...
yes maybe a big warning is the first step here
There was a problem hiding this comment.
you can revisit:
Preserve existing logic: skip when a type name already exists and overwrite_existing is False
Add warning when a discovered datatype is skipped
Add additional warning when the skipped entry has the same name but a different DataType NodeId
| @functools.cache | ||
| def _create_list_deserializer(uatype: type, recursive: bool = False) -> Callable[[Buffer | IO], list[Any]]: | ||
| if recursive: | ||
| def _create_list_deserializer(uatype: Any, recursive: bool = False) -> Callable[[Buffer | IO[Any]], list[Any]]: |
There was a problem hiding this comment.
I do not follow everything here but that typing change looks very strange. The entire point of that method is to take binary data and transform it into ojjects... hwy would it take Any??
There was a problem hiding this comment.
somehow typechecking blamed it... i looked it up:
Why not just IO?
In Python typing, IO is generic. Writing bare IO is considered incomplete by type checkers. IO[Any] means “an IO object of unspecified content type,” which is practical here because many real stream objects are not strictly annotated as IO[bytes] even though they return bytes at runtime.
There was a problem hiding this comment.
sorry somehow I missed the IO[] part ...
2 participants
fix for: #1940