A pull request for gnmic prompt mode

[neoul]

I hope that you accept my changes for gnmic prompt mode. The gnmic prompt mode is an interactive CLI to support the tab completion of the data path and enumeration values for user convenience. To use the tab completion of the gnmic prompt mode, the schema data of the user YANG data model should be generated before the prompt mode is loaded as the following examples.
The first example shows how to generate the schema data using the gnmic path subcommand. The schema data will be generated to $HOME/.gnmic.schema file if you execute the path command with --generate-schema option. once it generated, it will be loaded every prompt mode starting. The second example shows how to load the schema data directly from the YANG modules without gnmic path subcommand.

./gnmic path --dir ../gnmi/model/yang/ --file ../gnmi/model/yang/openconfig-interfaces.yang --file ../gnmi/model/yang/[email protected] --exclude ietf-interfaces --generate-schema
./gnmic --address 192.168.77.110:57400 --tls-ca ../gnmi/tls/ca.crt --tls-cert ../gnmi/tls/client.crt --tls-key ../gnmi/tls/client.key --username root --password admin -d prompt-mode

./gnmic --address 192.168.77.110:57400 --tls-ca ../gnmi/tls/ca.crt --tls-cert ../gnmi/tls/client.crt --tls-key ../gnmi/tls/client.key --username root --password admin -d prompt --dir ../gnmi/model/yang/ --file ../gnmi/model/yang/openconfig-interfaces.yang --file ../gnmi/model/yang/[email protected] --exclude ietf-interfaces

2020-10-05 Added prompt mode

• Added prompt mode subcommand to root command.
• Added the path completion in the prompt mode.
• Added ? key for the brief command help.
• Separated flag initialization to reset the used subcommand flags in the prompt mode.
• Updated path command to generate a schema file($HOME/.gnmic.schema) for tab completion of the prompt mode.
• Added --dir, --file and --exclude flags to the prompt mode for single step schema loading.
• Add command history to the prompt mode

Work Items remained for gnmic prompt mode

This prompt mode is still being developed. I think I can implement the first work item (The tab completion for boolean and enumeration values). But, the second and third items, they need to change the current framework. I hope that you check how to design and implement them.

1. boolean or enumeration values must be presented on the tab completion list upon set subcommand.
2. subcommand execution must return success or failure information to the terminal. (e.g. get doesn't any info upon get operation failure upon non-debug mode.)
3. Subscribe subcommand must be run in background and the prompt must be presented on the prompt mode. And the output flag configuration should be also supported for the gNMI telemetry streaming redirection in the prompt mode.

Thanks to check.!

neoul

[karimra]

Hi @neoul, great added features. Thanks for contributing.
This is a huge PR, please allow some time to read through and test it.

A couple of early questions:
a) Would it be possible to combine the rootCmd and promptCmd ?
Meaning if gnmic is ran without subcommands, it enters prompt mode. ( maybe via a --prompt? )
If this is too much work, don't worry about it, this can be addressed in a subsequent PR.

b) When in prompt mode, is there a way to still allow the user to set some global flags?

About item 3:

• Currently output definition is supported via config file only, we can consider a --output <output_name> flag to support this.
• Running the subscribe command in the background means we need to find a way to keep track of its state and be able to stop it.
  This means another set of subcommands needs to be added to the prompt command.
  I'm thinking subscription list, subscription show/state, subscription start, subscription stop

Great stuff, thanks again.
Karim

[neoul]

Answer about early questions:

a)
I am not sure it is possible. We have to check go-prompt that binds and runs within the cobra Run or RunE function. The separation of the commands (rootCmd and promptCmd) was the best choice to avoid the big change of the current command framework and much work. Could you let me know why you need the prompt flag, not the subcommand? (about your command concept)

b) It is possible now. you can see the global flags configuration if you type -- before typing subcommands in the prompt mode. I'd assume the global flags is already configured before the prompt mode starting. So, they are hid and not presented from the tab completion on which the subcommand is selected in the prompt mode.

gnmic> --
        --address             comma separated gnmi targets addresses            
        --config              config file (default is $HOME/gnmic.yaml)         
        --debug               debug mode                                        
        --encoding            one of [json bytes proto ascii json_ietf]. Ca...  
        --format              output format, one of: [protojson, prototext,...

About item 3:
Your idea is the same as I think!

[karimra]

a few minor changes and questions

[karimra]

about a) no specific reason for --prompt over prompt command, it just keeps the commands hierarchy clear
about b) ok, didn't notice that. but the global flags need to be given before the subcommand, while on shell mode the order does not matter. it's good enough this way :D

About items 1, 2 and 3, I think they can be added in future PRs

If I start gnmic in prompt mode with --dir and --file, the path command still requires a --file flag. Is it possible to set the flag path-file to the value of prompt-file if path is ran in prompt mode ?

[neoul]

If I start gnmic in prompt mode with --dir and --file, the path command still requires a --file flag. Is it possible to set the flag path-file to the value of prompt-file if path is ran in prompt mode ?

I think that would be possible.

[hellt]

Thanks for this awesome PR, @neoul
I'd like to discuss the YANG side of things

1. In your examples, why do you need to specify --file ../gnmi/model/yang/[email protected] when you import the opeconfig-interfaces module? I think goyang can dynamically find the imported modules
2. What is the reasoning behind excluding ietf-interfaces module?

[neoul]

Thanks for this awesome PR, @neoul
I'd like to discuss the YANG side of things

1. In your examples, why do you need to specify --file ../gnmi/model/yang/[email protected] when you import the opeconfig-interfaces module? I think goyang can dynamically find the imported modules

If we don't specify the iana-if-type.yang for --file option, we cannot get the identity schema data (e.g. ethernetCsmacd identity) defined in iana-if-type.yang from the goyang package. That identity schema data will be used for the tab completion suggestion on Set operation. (It is just a plan yet.)

2. What is the reasoning behind excluding ietf-interfaces module?

ietf-interfaces has the top node, interfaces. That is the same name to the top node in openconfig-interfaces and the ietf-interfaces module is only used for the type referencing in openconfig-interfaces module. So, this kind of referenced module should be excluded by using the --exclude option. This concept is coming from ygot package.

[hellt]

@neoul its strange about 1). I think goyang resolves identities when it builds the Entry by loading the modules dynamically.

At least when I build the paths with yangpath I do not need to pass any additional modules for goyang to succesfully build the entry with ms.Process() func. The reason I am after this is that I'd like the prompt process to be as easy as possible

2. can you please show me that concept in ygot if you have it somewhere?

[neoul]

@neoul its strange about 1). I think goyang resolves identities when it builds the Entry by loading the modules dynamically.
At least when I build the paths with yangpath I do not need to pass any additional modules for goyang to succesfully build the entry with ms.Process() func. The reason I am after this is that I'd like the prompt process to be as easy as possible

Try two following commands to check the difference. Find the type leaf in the search list. You can not get the type info if you don't set iana-if-type.yang.

gnmic path --dir ../gnmi/model/yang/ --file ../gnmi/model/yang/openconfig-interfaces.yang --file ../gnmi/model/yang/[email protected] --exclude ietf-interfaces --search

gnmic path --dir ../gnmi/model/yang/ --file ../gnmi/model/yang/openconfig-interfaces.yang --exclude ietf-interfaces --search

1. can you please show me that concept in ygot if you have it somewhere?

The following go generate command is used for generating ygot go structure. You can find exclude_modules in the command sample. It is almost the same to the '--exclude' flag of the prompt mode. The exclude_modules skips the code generation of the target module when it generate a go code.

go run github.com/openconfig/ygot/generator/generator.go -include_model_data -generate_fakeroot -output_file github.com/neoul/gnxi/gnmi/model/gostruct/generated.go -package_name gostruct -exclude_modules ietf-interfaces -path github.com/openconfig/public,github.com/YangModels/yang github.com/openconfig/public/release/models/interfaces/openconfig-interfaces.yang github.com/openconfig/public/release/models/openflow/openconfig-openflow.yang github.com/openconfig/public/release/models/platform/openconfig-platform.yang github.com/openconfig/public/release/models/system/openconfig-system.yang"

[neoul]

... oh.. sorry for my misclick. I'd closed it..

[karimra]

@neoul if the [email protected] file is under the directory in --dir, then there is no need to add it via a second --file right ?
and the exclude is to avoid double auto completion ?

[hellt]

Thanks @neoul
I was connfused because I didn't read the code at first =) I have a yangpath project with a similar goal (generate paths out of YANG modules), where I extract paths without ygot, just with goyang.

There I do path extraction out of a single module, therefore the modules that are required for type dereference or imported/included are not get in the way. The missing feature there was to understand the augmentation statements;

[hellt]

to my surprise, today in goyang repo the maintainers discussed the reasoning for making namespace-unaware merge of modules in ygot. That explains the need for -exlude openconfig/goyang#146 (comment)

So now I have the following mental notes in my head:

1. if we were to use only goyang to parse the modules, we would have avoided the need to make users aware of a clashing same-name root elements.
2. ygot uses namespace-unaware approach to simplify the developers experience by not including namespaces in the generated fund/struct names

[neoul]

@neoul if the [email protected] file is under the directory in --dir, then there is no need to add it via a second --file right ?

That was what I expected the result of goyang package. But, that is not resulted out. The identities of iana-if-type.yang is not presented without --file option. The identity type information of iana-if-type.yang is not shown. Find type leaf (/interfaces/interface[name=*]/config/type) using the following path command. That leaf would be empty.

gnmic path --dir ../gnmi/model/yang/ --file ../gnmi/model/yang/openconfig-interfaces.yang --exclude ietf-interfaces --search

and the exclude is to avoid double auto completion ?

Yes.

[neoul]

to my surprise, today in goyang repo the maintainers discussed the reasoning for making namespace-unaware merge of modules in ygot. That explains the need for -exlude openconfig/goyang#146 (comment)

So now I have the following mental notes in my head:

1. if we were to use only goyang to parse the modules, we would have avoided the need to make users aware of a clashing same-name root elements.

goyang creates top root elements for the module info and lists the schema data to a slice. So, crashing same-name root can be avoided. If we have to use the same-name root, the YANG prefix must be presented and processed in the path representation.

2. ygot uses namespace-unaware approach to simplify the developers experience by not including namespaces in the generated fund/struct names

Yes. you are right.

[neoul]

Thanks @neoul
I was connfused because I didn't read the code at first =) I have a yangpath project with a similar goal (generate paths out of YANG modules), where I extract paths without ygot, just with goyang.

There I do path extraction out of a single module, therefore the modules that are required for type dereference or imported/included are not get in the way. The missing feature there was to understand the augmentation statements;

The path is not a problem. The data I mean YANG identity value is not loaded.

[karimra]

LGTM,
@hellt if you don't have more questions let's merge this.

[hellt]

I'm happy to merge. The only outstanding piece from my end would be the missing resolved Identity-ref that needs to be looked at in a separate issue, as I wanted it to result either in a local fix or an issue against goyang/ygot

…

On Thu, 8 Oct 2020, 08:00 Karim Radhouani, ***@***.***> wrote: LGTM, @hellt <https://github.com/hellt> if you don't have more questions let's merge this. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#136 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABLKV5LIN6K4BFADOZ5437LSJVIQNANCNFSM4SEJ264A> .

[karimra]

yes, let's look at it in a separate issue. this is already a big PR