acidjunk / pyang

Automatically exported from code.google.com/p/pyang
ISC License
0 stars 0 forks source link

Deviations and DSDL plugin may not play well together #77

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
I believe I've found a bug related to deviations within pyang DSDL plugin. 
Deviations allow you to reference targets via absolute-schema-node-id which 
means that it can reference anything in the expanded children of a statement 
(one of the entries within Statement.i_children in pyang terms).

For some reason dsdl.py avoids using Statement.i_children all together. It has 
it's own “expansion phases” which take care of augmentations, refinements 
and grouping usages. Yet these fail to take deviations into account.

If I want to deviate a schema node which appears within the original statement 
structure of a module then it's probably going to be okay if I generate DSDL 
based on the deviated module. Pyang validation will simply change the original 
statement tree by applying the deviation. However, if I want to deviate a 
schema node which gets added to the schema tree via a grouping usage for 
example, then I'm going to have a bad time. The only place where the deviation 
will take effect is within Statement.i_children which never gets used within 
dsdl.py.

I've enclosed two sample modules which demonstrate the bug. One defines a 
schema tree and the other applies deviations to it. I used type-stmt replacing 
but this bug applies to any deviate-stmt which targets “virtual schema 
nodes”. If you use pyang to generate DSDL with these two modules, then the 
first deviation will fail to replace the type within hybrid schema and the 
second will succeed.

You should apply deviations the same way you do this for augmentation, 
refinement and grouping usage. The problem is that the original statement tree 
may have already been changed through deviations during module validation and 
you have no way of knowing that. Or you should somehow check if a statement has 
been deviated and create a new rng:define for it, instead of rng:ref-using the 
rng:define for a grouping, etc. Either way you cannot avoid using 
Statement.i_children.

I hope I succeeded in describing the problem. Deviations suck! :)

P.S.: the enclosed module which defines the schema tree has a dependency. 

Original issue reported on code.google.com by jernej.t...@gmail.com on 12 Sep 2012 at 11:25

Attachments:

GoogleCodeExporter commented 9 years ago

Original comment by mbj4...@gmail.com on 12 Sep 2012 at 12:03

GoogleCodeExporter commented 9 years ago
I have to check what's happening. The DSDL plugin relies completely on the 
parser to apply all deviations to the statement tree. The 'deviation' statement 
itself is ignored.

Original comment by lada.lho...@gmail.com on 12 Sep 2012 at 1:06

GoogleCodeExporter commented 9 years ago
I think relying on the validator to do that for you was a mistake. You cannot 
apply all deviations on the original statement tree because some nodes don't 
even exist in it. You'd have to replace all uses-stmts with corresponding 
grouping-stmt content, for example, in order to achieve that. That's why each 
schema node in the original statement tree has an i_children member. By 
traversing them instead of the original statement's schema node children, you 
can traverse the entire schema tree of a module. This schema tree may contain 
references to statements of the original statement tree, copies of 
grouping-stmt substatements, references to augmented nodes, etc. And this is 
what you should be traversing instead of the original statement tree (at least 
partially).

I think Martin will confirm this but I'm pretty sure that's how it is 
implemented...I had to visualise it in a GUI application.

Original comment by jernej.t...@gmail.com on 12 Sep 2012 at 1:53

GoogleCodeExporter commented 9 years ago
I can confirm your observations.

The DSDL plugin does not use the i_children because the hybrid schema also 
keeps the tree partially unexpanded - YANG groupings and typedefs are mapped to 
RELAX NG named patterns if possible.

As you say, I can implement deviations in the DSDL plugin analogically to 
augments and refinements, but only if the parser does not apply deviations in 
the main statement tree. That is, deviated versions of all nodes should be 
accessible only via i_children.

Original comment by lada.lho...@gmail.com on 13 Sep 2012 at 12:44

GoogleCodeExporter commented 9 years ago
While you are at it, I noticed something else related to refinements but am 
unsure if it is a bug or a feature. If a description or reference get refined, 
then this means that they replace the originals, yet pyang DSDL plugin adds 
both original and refined texts as documentation to a hybrid schema node. Is 
this intentional?

Not really worth to be posted as a new issue, so I'm posting it here, since 
this is a side-effect of process_patches(...) function, which you are bound to 
change if you plan to implement deviations the same way you do refinements and 
augments. 

Original comment by jernej.t...@gmail.com on 21 Sep 2012 at 12:26

GoogleCodeExporter commented 9 years ago
Yup, that's how I understood the sentence in sec. 7.12.2 of 6020 that any node 
may get a *specialized*, rather than *different*, "description and reference 
statement. Do you think it is a problem? 

Original comment by lada.lho...@gmail.com on 21 Sep 2012 at 12:45

GoogleCodeExporter commented 9 years ago
No, it's not a problem. It doesn't have much impact on the actual model 
anyways. 

Original comment by jernej.t...@gmail.com on 21 Sep 2012 at 1:07