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.

Sign up for GitHub

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

Jump to bottom

No access to meta.List's key when module defines list in augment #110

Open
berupp opened this issue Feb 29, 2024 · 14 comments
Open

No access to meta.List's key when module defines list in augment #110

berupp opened this issue Feb 29, 2024 · 14 comments

Comments

@berupp
Copy link

berupp commented Feb 29, 2024

I am trying to work with the AST of a module. I am loading the following YANG using parser.LoadModule

module myModule {

    yang-version "1.1";
	
    namespace "http://myModule.com/ns/yang/eth_dm";
	
    prefix "myModule";
   
    organization "myOrg";
 
    revision 2024-02-26 {
        description
            "Newly generated revision";
        reference "1.0.0";
    }
       augment "/otherModule:object-type" {
	    when "/otherModule:object-type/otherModule:typename = 'eth-dm'";
		container eth-dm {
			uses monitored-objects;
		}
	}

	grouping monitored-objects {
		container monitored-objects {
			list monitored-object {
				key "monitored-object-id monitored-object-name";
			}
		}
	}
}

Trying to access they key of the monitored-objects list is not possible

module.Groupings()["monitored-objects"].DataDefinitions()[0].(*meta.Container).DataDefinitions()[0].(*meta.List).KeyMeta()

is returning nil.

The private key property of meta.List is populated, but I noticed that the entire container monitored-objects is not compiled because it is only referenced by the augment and not a by container rooted in the module. Therefore, the code which is populating the keyMeta property of meta.List isn't executed. Which also means that I can provide a key that is referencing a non-existent leaf without getting any compile errors on LoadModule.
@dhubler
Copy link
Collaborator

dhubler commented Feb 29, 2024 via email

@berupp
Copy link
Author

berupp commented Feb 29, 2024 •
edited
Loading 

p := source.Path(directoryPath)
module, err := parser.LoadModule(p, "myModule")
if err != nil {
	return fmt.Errorf("unable to load module %s: %w", "myModule", err)
}

The grouping is used in the

container eth-dm {
	uses monitored-objects;
}

which is defined in the augment.

It seems the augment is parsed but not compiled at all

@dhubler
Copy link
Collaborator

dhubler commented Mar 1, 2024 via email

@dhubler
Copy link
Collaborator

dhubler commented Mar 1, 2024 via email

@berupp
Copy link
Author

berupp commented Mar 1, 2024

Thank you so much for looking into this!

If I move c1 into original module and get rid of import, I get the error

I have made the same observation. But the way my models are structured is to have one module (the main module) with a container object-type and then I have separate models for each object type that define an augment for said object-type container in the main module, just like you have in your example above (c1).

There are other things defined in the main module (typedefs, common groupings etc). I like this structure, because it allows me to add more object types without forcing consumers to recompile the main module, as they'd would have to do if I used a module-submodule relationship.

It's there a reason you're trying to get keys on grouping and not final container?

I don't know how. My models look pretty much like your example. If I load issue-110-other, it doesn't know about the augment in issue-110 and any of its definitions.

If I load issue-110, it doesn't know about c1. I only have access to the groupings, augment etc defined in issue-110.

Is there a way to load issue-110-other and issue-110 as a whole? And the basically have c2 appear under c1? I looked at the API but none of the parser.LoadModuleXXX functions seems to support that

@dhubler
Copy link
Collaborator

dhubler commented Mar 2, 2024 via email

@berupp
Copy link
Author

berupp commented Mar 4, 2024 •
edited
Loading

The "augment" statement allows a module or submodule to add to a
schema tree defined in an external module, or in the current module
and its submodules

Based on my interpretation, augments exist for the purpose of monkey patching, especially when it comes to external modules.

Otherwise, the above suggestion isn't bad. It just removes the advantage of using an augment versus using a parent module with submodules for each object type, because I will have to release the parent module after each addition of an object type.

Would it be feasible to add a function Load to parser that allows loading multiple YANG files? That would allow the application of the augment to container c1 and be present in the compiled module?

@dhubler
Copy link
Collaborator

dhubler commented Mar 5, 2024 via email

@berupp
Copy link
Author

berupp commented Mar 6, 2024

Yes pyang parses the model flawlessly.
I am not posting the full model here because proprietary :) But let's take your example, which essentially looks like mine in a nutshell:

module issue-110-other {
    yang-version "1.1";
    namespace "http://freeconf.org/ns/issue-110-other";
    prefix "i";

    container c1 {
        leaf l2 {
            type string;
        }
    }
}

module issue-110 {
    yang-version "1.1";
    namespace "http://freeconf.org/ns/issue-110";
    prefix "i";

    import issue-110-other {
        prefix "o";
    }

    augment "/o:c1" {
        when "/o:c1/o:l2 = 'x'";
        container c3 {
            uses g1;
        }
    }

    grouping g1 {
        container c2 {
            list l1 {
                key "k1 k2";
                leaf k1 {
                  type string;
                }
                leaf k2 {
                  type string;
                }
            }
        }
    }
}

The result of pyang -f tree * (with both yang files in the directory)

module: issue-110-other
  +--rw c1
     +--rw l2?     string
     +--rw i:c3
        +--rw i:c2
           +--rw i:l1* [k1 k2]
              +--rw i:k1    string
              +--rw i:k2    string

@dhubler
Copy link
Collaborator

dhubler commented Mar 7, 2024 via email

@berupp
Copy link
Author

berupp commented Mar 7, 2024 •
edited
Loading

If I was developing a device I
would need to pick one module or the other, not the merging of multiple
modules unless I am missing something

Let's say you device supports module issue-110-other. But the augmented version, where the augment is defined in issue-110, then device supports:

module: issue-110-other
  +--rw c1
     +--rw l2?     string
     +--rw i:c3
        +--rw i:c2
           +--rw i:l1* [k1 k2]
              +--rw i:k1    string
              +--rw i:k2    string

It makes complete sense to me. Because I understand augments as monkey patch to other existing modules. I think that is where our understanding is different. Once you see an augment as an addition to an existing module, which could or could not be supported by certain devices, this all makes sense

@berupp
Copy link
Author

berupp commented Mar 25, 2024

Would it be possible to add an accessor for the private field keys in *meta.List so I have access to the defined keys. This would fix they issue I am currently having without 'compiling' the module.

Right now the keys are in a private field, with no accessor

@dhubler
Copy link
Collaborator

dhubler commented Mar 26, 2024 via email

@berupp
Copy link
Author

berupp commented Mar 27, 2024 •
edited
Loading

Screenshot 2024-03-27 at 9 47 01 AM

It is not in keyMeta because the augment is not compiled. This is my problem that I outlined in the initial comment on this issue.

So problem

  1. I do not have access to the key. This I really need, I need access to the defined keys of a list. I forked and added a simple accessor and it works, but I'd rather not fork :)
  2. Due to the lack of execution of freeconf's 'compile' (for a augment structure as discussed above), I can technically add key definitions that have no leaf representation and freeconf/yang is fine with it. This should be fixed IMO, but isn't really a pressing issue for me. If the 'compile was executed, keyMeta would be populated as well.

By adding the accessor for the key field in *meta.List, you'd make my day :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dhubler @berupp