Skip to content
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

Xml improved 2 #94

Merged
merged 4 commits into from
Feb 22, 2021
Merged

Xml improved 2 #94

merged 4 commits into from
Feb 22, 2021

Conversation

kwatsen
Copy link
Contributor

@kwatsen kwatsen commented Feb 11, 2021

Hi Lada,

Please merge and publish update.

Thanks,
Kent

kwatsen and others added 4 commits January 8, 2021 13:29
Fix notification generation and remove allow_nodata parameter
Fix detection of no text for empty type
Simplify XML handling and cleanup notification usecase
@llhotka
Copy link
Member

llhotka commented Feb 11, 2021

A documentation test fails for orphan_instance method:

**********************************************************************
File "schemanode.rst", line 427, in default
Failed example:
    obag['foo'].value
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python3.9/doctest.py", line 1336, in __run
        exec(compile(example.source, filename, "single",
      File "<doctest default[2]>", line 1, in <module>
        obag['foo'].value
      File "/home/lhotka/Projects/yangson/yangson/instance.py", line 216, in __getitem__
        return self._member(key)
      File "/home/lhotka/Projects/yangson/yangson/instance.py", line 584, in _member
        raise NonexistentInstance(self.json_pointer(),
    yangson.exceptions.NonexistentInstance: [/] member 'foo'
**********************************************************************

@kwatsen
Copy link
Contributor Author

kwatsen commented Feb 11, 2021

@HRogge, how can this be related to the PR?

@llhotka, how to debug? I installed Sphynx and can reproduce running make doctest, but I can't instrument the code to see what's going on without Sphynx blowing up. What's the easiest way to isolate and/or reproduce the test outside Sphynx?

@kwatsen
Copy link
Contributor Author

kwatsen commented Feb 11, 2021

Creating yangson/docs/examples/ex4/test_pr94.py as follows:

import json
from yangson import DataModel

dm = DataModel.from_file('yang-library-ex4.json', mod_path=['.', '../../../yang-modules/ietf'])
fsn = dm.get_schema_node('/example-4-a:bag/foo')
rsn = dm.get_schema_node('/example-4-a:bag/opts/example-4-b:fooref/fooref')
with open('example-data.json') as infile:
    ri = json.load(infile)
inst = dm.from_raw(ri)
bsn = rsn.data_parent()
obag = bsn.orphan_instance({'foo': 54, 'bar': True})
assert(obag.name == 'example-4-a:bag')
assert(obag['foo'].value == 54)
assert((obag.parinst is None) == True)
assert(obag.siblings == {})

and running python test_pr94.py reproduces the result reported by Lada.

Annotating InstanceNode:_member() as follows:

    def _member(self: "InstanceNode", name: InstanceName) -> "ObjectMember":
        print("NEW Inside InstanceNode:_member()...")
        print("name = " + name)
        pts = name.partition(":")
        print("pts = " + str(pts))
        print("self.namespace = " + self.namespace)
        if pts[1] and pts[0] == self.namespace:
            name = pts[2]
            print("new name = " + name)
        sibs = self.value.copy()
        print("sibs = " + str(sibs))
        print("self._member_schema_node(name) = " + str(self._member_schema_node(name)))
        print("sibs.pop(name) = " + str(sibs.pop(name)))
        print("DONE")
        ...

and running again produces this output:

Inside InstanceNode:_member()...
name = foo
pts = ('foo', '', '')
self.namespace = example-4-a
sibs = {'example-4-a:foo': 54, 'example-4-a:bar': True}
self._member_schema_node(name) = <yangson.schemanode.LeafNode object at 0x10b559820>
Traceback (most recent call last):
  File "test_pr.py", line 19, in <module>
    assert(obag['foo'].value == 54)
  File "/Users/kent/Code/GitHub/kwatsen/yangson/yangson/instance.py", line 216, in __getitem__
    return self._member(key)
  File "/Users/kent/Code/GitHub/kwatsen/yangson/yangson/instance.py", line 586, in _member
    print("sibs.pop(name) = " + str(sibs.pop(name)))
KeyError: 'foo'

Running the exact same code on yangson/master branch produces this output:

NEW Inside InstanceNode:_member()...
name = foo                                                 
pts = ('foo', '', '')                                      
self.namespace = example-4-a                               
sibs = {'foo': 54, 'bar': True}
self._member_schema_node(name) = <yangson.schemanode.LeafNode object at 0x110324700>
sibs.pop(name) = 54
DONE
...

Notice the difference?

OLD: sibs = {'foo': 54, 'bar': True}
NEW: sibs = {'example-4-a:foo': 54, 'example-4-a:bar': True}

@HRogge: thoughts?

@llhotka
Copy link
Member

llhotka commented Feb 12, 2021

The name of the orphan instance is qualified with the module name (example-4-a:bag), so its members shouldn't be.

@kwatsen
Copy link
Contributor Author

kwatsen commented Feb 17, 2021

@HRogge I really don't know how to begin fixing this one, can you look at this while I fix Issues #86, #91, and #95?

@llhotka
Copy link
Member

llhotka commented Feb 19, 2021

I tried to fix this issue with orphan_instance but the test now fails on a notification instance. I pushed these changes to this PR branch, so you can see yourself.

These problems boil down to the fact that XML representation in RESTCONF requires an encapsulating element and instances of schema nodes are chosen rather arbitrarily for this purpose: it is an InputNode/OutputNode for rpcs/actions but NotificationNode for notifications.

I also tried to make NotificationNode a subclass of InternalNode rather than SchemaTreeNode but something else fails again. So I really don't know what to do with this.

Everything would be quite easy if RESTCONF provided a neutral data encapsulation container such as restconf:data. Too late.

:-\

@kwatsen
Copy link
Contributor Author

kwatsen commented Feb 19, 2021

Thank you for taking a look!

Yes, too late to fix RESTCONF (unless it's in restconf-next) . [file an issue there?]

Maybe @HRogge has an idea, since he submerged himself in all this before?

Another fix, though much more invasive, is to modify Yangson to have each object representation to include its own envelop, like RESTCONF. That is, GET/PUT/etc. operations on resource "foo" always include envelop { foo: { <children> } } whereas Yangson operations are all of the form { <children> }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants