Feature #3864
closed
nfdc face commands
Added by Junxiao Shi over 7 years ago. Updated about 7 years ago.
100%
Description
In nfdc, implement these commands:
face list [[remote] <uri>] [local <uri>] [scheme <scheme>]
face show [id] <id>
face create [remote] <uri> [[persistency] <persistent|permanent>]
face destroy [id] <id>
face destroy [remote] <uri>
channel list
New commands should have #2542 output format; fix any IntegrationTests issues caused by face list format change.
Deprecate create and destroy commands, but do not change their output format.
Files
| 20170126182542.tgz (836 KB) 20170126182542.tgz | integ NFD 3359,9 | Junxiao Shi, 01/26/2017 12:01 PM |
Updated by Junxiao Shi over 7 years ago
- Related to Feature #2542: Redesign nfdc combining nfdc and nfd-status added
Updated by Junxiao Shi over 7 years ago
- Status changed from New to In Progress
- Assignee set to Junxiao Shi
Updated by Junxiao Shi over 7 years ago
- % Done changed from 0 to 20
https://gerrit.named-data.net/3359 implements face show command.
patchset1 focuses on output format, and only handles successful case; I'll handle failure case in a later patchset.
Example output:
vagrant@m0212:~/NFD$ nfdc face show 258
faceid=258
remote=fd://25
local=unix:///run/nfd.sock
counters={in={5510i 14d 0n 291775B} out={14i 968d 0n 636549B}}
flags={local on-demand point-to-point local-fields}
vagrant@m0212:~/NFD$ nfdc face list | grep faceid=258
faceid=258 remote=fd://25 local=unix:///run/nfd.sock counters={in={5521i 14d 0n 292337B} out={14i 972d 0n 638396B}} flags={local on-demand point-to-point local-fields}
While the design suggests a key-value pair format for each attribute with a space in between, I decide to keep the existing {} style since it shouldn't cause much difficulty for script consumption as long as parentheses are paired correctly.
Attributes are aligned at =, which is consistent with existing status show command.
I moved local on-demand point-to-point into flags to satisfy key-value pair format. While they are technically not flags in FaceMgmt protocol, from user point of view they are no different from flags, thus it makes sense to list them together with local-fields. This change applies to nfdc face list command as well, since they are generated from the same code.
Updated by Davide Pesavento over 7 years ago
[...] it shouldn't cause much difficulty for script consumption [...]
I don't think "script consumption" should be a design goal for the human-readable output format. If you want an easy-to-parse (and hopefully stable) format, implement JSON or XML output.
Updated by Junxiao Shi over 7 years ago
I don't think "script consumption" should be a design goal for the human-readable output format. If you want an easy-to-parse (and hopefully stable) format, implement JSON or XML output.
I want to achieve a balance between human-reabable and machine-readable. The old format contains too many spaces which is very difficult for script consumption because any small change in the output almost always breaks scripts. This is also why I have hyphens in command result like face-created or face-not-created.
Updated by Davide Pesavento over 7 years ago
Junxiao Shi wrote:
I want to achieve a balance between human-reabable and machine-readable. The old format contains too many spaces which is very difficult for script consumption because any small change in the output almost always breaks scripts.
As I said, machine-readability is (or should be) a non-goal. We never made any guarantees of stability on the current output format, and we shouldn't make any for the "new" format either (note, that doesn't mean we will gratuitously change it at every occasion). If you want programmatic parseability, you should use a structured format with precise syntax rules, so that a parser can be written (or generated) for it. And at that point, I strongly suggest using an existing format, such as JSON or XML.
This is also why I have hyphens in command result like face-created or face-not-created.
Well I think you're doing it wrong. To check for success/failure you should simply check the exit code of nfdc, i.e. $? from a shell script.
Updated by Junxiao Shi over 7 years ago
I don't want to go into a fully machine readable format, but only a balance between human readable and machine readable, without promising stableness. I'm already doing so with many of the outputs in strategies.
To determine whether a face is successfully created, exit code is the primary measure. However, using face-created as a single token still alows easier parsing, without sacrificing human readability.
Updated by Davide Pesavento over 7 years ago
Junxiao Shi wrote:
I don't want to go into a fully machine readable format, but only a balance between human readable and machine readable, without promising stableness. I'm already doing so with many of the outputs in strategies.
If there's no stability promise, then fine.
Updated by Junxiao Shi over 7 years ago
https://gerrit.named-data.net/3359 patchset3 gives my idea on how to handle dataset errors. Please review.
I'll add unit testing after this design is reviewed.
Updated by Junxiao Shi about 7 years ago
- % Done changed from 20 to 30
https://gerrit.named-data.net/3359 adds unit tests. It required quite a bit of refactoring.
Updated by Junxiao Shi about 7 years ago
- File 20170126182542.tgz 20170126182542.tgz added
Updated by Junxiao Shi about 7 years ago
- % Done changed from 30 to 50
https://gerrit.named-data.net/3633 adds face create command.
An FindOrCreateFace component is introduced, given that other commands in RIB etc will need to find or create faces as well; this is intended to be the replacement of FaceIdFetcher.
Updated by Junxiao Shi about 7 years ago
- % Done changed from 50 to 70
https://gerrit.named-data.net/3661 adds face destroy command.
FindFace::execute(const FaceQueryFilter&, ..) overload is not strictly necessary for this command, but it's needed for face list [[remote] <uri>] [local <uri>] [scheme <scheme>].
A consequence is that FaceUri is first converted to string, and then parsed back from string.
I wonder why FaceQueryFilter stores strings instead of FaceUris?
Updated by Junxiao Shi about 7 years ago
https://gerrit.named-data.net/3670 adds == operator to ndn::nfd::FaceQueryFilter, which can simplify unit testing of some nfdc commands.
Updated by Junxiao Shi about 7 years ago
https://gerrit.named-data.net/3674 fixes a typo in the FaceQueryFilter == operator change which prevents it from working.
https://gerrit.named-data.net/3675 uses FindFace in face show command.
Updated by Junxiao Shi about 7 years ago
- % Done changed from 70 to 80
https://gerrit.named-data.net/3681 rewrites face list to support filtering. The implementation is based on FindFace and is separate from FaceModule::fetchStatus. FaceModule::fetchStatus is still needed for use by status report.
However, patchset1 is not working with real NFD: FindFace::canonize is stuck within face.processEvents(), possibly related to #1785. I'm still investigating.
face destroy is also not working when invoked with FaceUri due to the same FindFace::canonize problem. I will hold off adding more commands relying on FindFace::canonize until this is fixed.
Note that face show is unaffected despite it uses FindFace, because it only accepts FaceId.
The current FindFace::query implementation always fetches faces/query dataset even if the filter is empty. It would be more efficient to fetch faces/list dataset. This optimization will be added in a later patchset or another commit. https://gerrit.named-data.net/3683 adds FaceQueryFilter::empty() function to support this optimization.
face create is not working with real NFD: I thought faces/create command response contains RemoteUri (and assumed so in unit testing) but it doesn't. https://gerrit.named-data.net/3682 fixes this bug.
Updated by Junxiao Shi about 7 years ago
- Blocks Feature #3956: faces/create response: LocalUri field added
Updated by Junxiao Shi about 7 years ago
FindFace::canonize problem is found to be caused by Bug #3957. I do not know how to fix that bug. https://gerrit.named-data.net/3685 adds a workaround in FindFace::canonize. I have tested that face destroy and face list with FaceUri both work properly with real NFD.
Updated by Junxiao Shi about 7 years ago
- Status changed from In Progress to Code review
- % Done changed from 80 to 100
https://gerrit.named-data.net/3704 deprecates nfdc create and nfdc destroy. These commands print a deprecate notice to stderr when used.
nfdc(1) manpage is kept unchanged. It will be rewritten after all legacy nfdc commands are deprecated.
Updated by Junxiao Shi about 7 years ago
- Status changed from Code review to Closed