Fixes #179: Ensure uniqueness in leaf-list #200
Conversation
Codecov Report
@@ Coverage Diff @@
## master #200 +/- ##
==========================================
- Coverage 73.11% 70.19% -2.92%
==========================================
Files 7 7
Lines 1767 1661 -106
Branches 559 459 -100
==========================================
- Hits 1292 1166 -126
- Misses 353 369 +16
- Partials 122 126 +4
Continue to review full report at Codecov.
|
| @@ -377,19 +377,25 @@ class TypedList(collections.MutableSequence): | |||
| _list = list() | |||
|
|
|||
| def __init__(self, *args, **kwargs): | |||
| self._unique = kwargs.pop('unique', False) | |||
There was a problem hiding this comment.
Is there ever a scenario that we expect that a list is not unique? If not, it might be best to make this flag work the other way around such that we have something that says to skip the uniqueness check?
| @@ -844,7 +844,7 @@ def get_children(ctx, fd, i_children, module, parent, path=str(), | |||
| # TypedList (see lib.yangtypes) with a particular set of types allowed. | |||
| class_str["name"] = "__%s" % (i["name"]) | |||
| class_str["type"] = "YANGDynClass" | |||
| class_str["arg"] = "base=" | |||
| class_str["arg"] = "unique=True, base=" | |||
There was a problem hiding this comment.
Having the unique the other way around in TypedListType would mean that we could avoid this in the generated code?
pyangbind/lib/yangtypes.py
Outdated
| @@ -377,7 +377,7 @@ class TypedList(collections.MutableSequence): | |||
| _list = list() | |||
|
|
|||
| def __init__(self, *args, **kwargs): | |||
| self._unique = kwargs.pop('unique', False) | |||
| self._unique = kwargs.pop('unique', True) | |||
There was a problem hiding this comment.
Per the comments in Slack. I'm a little torn on this one.
If we make the default here True, then we'll end up saying that if existing consumers just upgrade pyangbind module from PyPi, but don't regenerate bindings, then existing code that expects this to be the previous state will end up being broken.
Sorry for the hassle, but I think requiring regenerating bindings such that the feature is turned on as you'd had this originally is safer to allow consumers to choose when to enable this.
There was a problem hiding this comment.
@tarkatronic reverted the commit with the outstanding comments in. LGTM!
Fixes #179
I couldn't use the
set()type directly due to the fact they do not maintain order. So instead, I've added aunique=Trueoption, along with a uniqueness check on value assignments toTypedList.