[Nexthop] Implement distributed command registration system#1016
Open
manoharan-nexthop wants to merge 2 commits intofacebook:mainfrom
Open
[Nexthop] Implement distributed command registration system#1016manoharan-nexthop wants to merge 2 commits intofacebook:mainfrom
manoharan-nexthop wants to merge 2 commits intofacebook:mainfrom
Conversation
…individual files Move explicit template instantiations from the monolithic CmdHandlerImplConfig.cpp file to individual command .cpp files. This is the first step in refactoring the CLI command registration system to avoid creating a monolithic include file.
Currently the `CommandTree` is statically built with all the commands and their actions defined in a single file `CmdListConfig.cpp` / `CmdList.cpp`. This makes it difficult to maintain over time and with all the commands included in a single file, makes the compilation slow.
Proposed mechanism:
Each CLI will do the registration of the CLI statically from within its definition file. As an Example,
```
namespace {
facebook::fboss::CommandDef<facebook::fboss::CmdConfigVlanStaticMac>
registerConfigVlanStaticMac(
"config vlan static-mac",
"Configure VLAN static MAC");
} // namespace
```
When the above is done, the `CommandDef` class invokes `CommandRegistry::registerCommand()` to add the command to a `vector` at the start of the CLI process.
And when the CLI initialization is done invoking `kCommandTree()`, it invokes `CommandRegistry::buildTree()` to create the sorted tree by splitting the command string and creating nodes for them.
Since the commands are registered (added to `vector` at random order), `config vlan static-mac` could come before `config vlan`, the `buildTree` takes care of handling them gracefully based on the handler registration information for each CLI.
There are 3 registration types that are introduced:
| Type | Use Case | Handlers |
|------|----------|----------|
| CommandDef<CmdType> | Commands with handlers (most commands) | Auto-inferred from CmdType |
| CommandDefNoHandler | Leaf commands with no children and no handler | None |
| AttributeDef | Simple attributes handled by parent | Only argTypeHandler |
NOTE: Parent commands (commands with children) must use CommandDef<CmdType> (as it is required to handle the depth counter in argument parsing).
1. All the Unit tests passes
2. Ran the CLI end to end tests and they pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Currently the
CommandTreeis statically built with all the commands and their actions defined in a single fileCmdListConfig.cpp/CmdList.cpp. This makes it difficult to maintain over time and with all the commands included in a single file, makes the compilation slow.Proposed mechanism:
Each CLI will do the registration of the CLI statically from within its definition file. As an Example,
When the above is done, the
CommandDefclass invokesCommandRegistry::registerCommand()to add the command to avectorat the start of the CLI process.And when the CLI initialization is done invoking
kCommandTree(), it invokesCommandRegistry::buildTree()to create the sorted tree by splitting the command string and creating nodes for them.Since the commands are registered (added to
vectorat random order),config vlan static-maccould come beforeconfig vlan, thebuildTreetakes care of handling them gracefully based on the handler registration information for each CLI.There are 3 registration types that are introduced:
NOTE: Parent commands (commands with children) must use CommandDef (as it is required to handle the depth counter in argument parsing).
Test Plan