resolve nsu= URIs in browse_root, add browse_cache_ttl - #132
Conversation
caterryan
left a comment
There was a problem hiding this comment.
The core design is sound. I verified: the deep copy keeps the original nsu= config, uri_to_index is fully built before any node ID changes, and each reconnect re-resolves against the live namespace array. The items below block merge. Two of them are on lines outside the diff, so they are here in the body.
1. Error masking — fix this first. opcua.py L467-468 sets _ENABLE_FULL_LOGGING from the config key before _validate_toml_nodes (L495) and _validate_toml_browse (L499) run. When the key is absent, the value becomes False, so every later validation error logs only ValueError. This defeats the default-True intent that the comment at L229-231 states. The CLI path has the same problem at L784-788: the assignment runs before the rest of argument parsing. Move both assignments to after validation completes. This bug hides the error text for every other problem in this list.
2. Raw exception text at L1866. _read_with_reconnect logs {e} directly. Use _exc(e) so the log does not leak details when full logging is off. The PR touches these paths, and this is the last raw-exception log in them.
3. Add a minimal test file. This PR adds cache-lifecycle and namespace-resolution logic, and four sibling plugins already have a test_*.py. Minimum set: _split_nsu_node_id cases (include a %3B URI and an empty URI), _validate_positive_int bounds, and one assertion with a fake cache that cache.put("opcua_config", ...) receives the expected TTL.
4. Document the TOML coercion change. _validate_positive_int coerces numeric strings, so browse_depth = "2" in TOML is now accepted where main rejects it. Add one README line that states this, so the behavior change is deliberate and visible.
Closes #131
Closes #130