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

Notification.from_raw() difficult to use. #78

Open
kwatsen opened this issue Dec 19, 2020 · 3 comments
Open

Notification.from_raw() difficult to use. #78

kwatsen opened this issue Dec 19, 2020 · 3 comments

Comments

@kwatsen
Copy link
Contributor

kwatsen commented Dec 19, 2020

Looking at the test_xml_notification pytest added to test_models.py in my xml-improved fork of yangson.

Notice that it works okay when from_raw() is passed this object:

{
    "testb:leafO" : True
}

But not this (more common) object:

{
    "testb:noA" : {
        "leafO" : True
    }
}

I understand that this is how Yangson generally works, but I wish that it were more like RESTCONF, whereby the object passed in and out of API calls has its top-level node being the node itself (not just it's contents).

The reason I care is because clients will always send notifications (i.e., via RFC 8040 SSE or the https-notif draft) with the top-level node existing, and all default-namespace settings are applied there. As it stands, the server-logic needs to float the default namespace down a level, without clobbering any already set default namespaces.

BTW, I tried toggling the allow_nodata boolean parameter, but it had no effect. To be honest, I'm still unclear what the allow_nodata is supposed to do. Maybe @HRogge can say? Is it, by chance, to allow exactly what I want (i.e., to toggle whether if the top-level node is included or not?)

FWIW, this issue also applies to InputNode and OutputNode, but they are both under RpcActionNode, whose from_raw() gives the expected behavior (i.e., a document with top-level node like {prefix}input. That said, it does also allow a document to have both an 'input' and 'output' simultaneously, a scenario that should never occur in normal operation.

Please respond quickly, as I'm hoping to get the xml-improved fork merged into master early next week. I'll submit a PR once I tidy up a few things, like adding a test_error pytest, etc.

@HRogge
Copy link
Contributor

HRogge commented Jan 6, 2021

allow_no_data was a "hotfix" for the issue that Input/Output nodes of RPCs are not data-nodes... so "get_data_child()" would skip this nodes which have to be present when parsing a RPC request. When I first requested making Input/OutputNode derive from DataNode it was not accepted, so I got this "switch" merged.

it seems we forgot to remove the allow_nodata hotfix when we decided to derive Input/OutputNode from DataNode. I just tested it with my Restconf-Code, I don't need allow_nodata anymore for RPC access.

I have yet to look into NotificationNode and its codepath.

@HRogge
Copy link
Contributor

HRogge commented Jan 6, 2021

To do what you want we would need the equivalent of the "is_root" parameter for from_raw().
this tells "from_xml()" that is should encode the current node, not its children... the rest is then solved by recursion.

@kwatsen
Copy link
Contributor Author

kwatsen commented Feb 23, 2021

I was looking to close this issue, but noticed that the test_xml_notification pytest in test_models.py was rather sparse and so spent some time trying to improve it...

As a recap, the goal of this test is to roundtrip (JSON --> XML --> JSON) a notification in order to exercise all the various bits.

Unfortunately, I wasn't able to do much of anything, as I continually ran into NonexistentSchemaNode: testb:noA under / and SchemaError: [/] member-not-allowed: testb:noA exceptions.

PS: the following sections should be removed now:

    ###################
    # JSON - RESTCONF # 
    ###################
    <snip/>

    ######################
    # JSON - HTTPS-NOTIF #
    ######################
    <snip/>

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

No branches or pull requests

2 participants