Attention is currently required from: fixeria. neels has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/osmo-upf/+/37830?usp=email )
Change subject: add VTY 'gtp-echo' command ...................................................................... Patch Set 1: (3 comments) File src/osmo-upf/upf_vty.c: https://gerrit.osmocom.org/c/osmo-upf/+/37830/comment/d3d76587_487cbd22?usp=email : PS1, Line 491: tx to > The command vector looks overloaded to me: […] Would you accept this? gtp-echo tx remote-ip IP_ADDR [(local-ip|local-dev)] [IP_ADDR_OR_DEV] The reason i sprinkle these words in between is to allow future extensions to the vty language. When arguments have no explicit prefix like 'gtp-echo REMOTE_ADDR', then future 'gtp-echo' commands are prone to have conflicts. What if we want to allow resolving a hostname (stupid example but..). What then if a user wants to use a hostname that is the same as some other vty token that an alternative command offers in the same place. I've faced such situations before, so I would much prefer to explicitly name each argument. I agree that 'tx to' doesn't seem nice, but think semantically, inserting 'foo' for arbitrary future extensions: ``` gtp-echo |- gtp-echo tx | |- to | |- from | | |- ip | | |- dev | |- foo |- gtp-echo foo ``` That is my idea behind it: the "tx" is one branch, the "to" a sub-branch of the argument logic. I thought somewhat about this but would like to keep those keywords in there. Is that too future paranoid? https://gerrit.osmocom.org/c/osmo-upf/+/37830/comment/74265275_b829a746?usp=email : PS1, Line 556: %%Error > cosmetic: missing space missing space? File tests/gtp-echo.vty: https://gerrit.osmocom.org/c/osmo-upf/+/37830/comment/e9563e63_dde20d69?usp=email : PS1, Line 2: show pid : show uptime : show version : show online-help : list [with-flags] : exit : help : show vty-attributes : show vty-attributes (application|library|global) : enable [expert-mode] : terminal length <0-512> : terminal no length : > these lines (and more below), for obvious reasons, shall not be part of the > VTY test oof, got so annoyed from removing them every time that i just left them in while testing ... and forgot! thanks -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/37830?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: I970dccd7a27b098eea9e660822e24e2c4b059fc6 Gerrit-Change-Number: 37830 Gerrit-PatchSet: 1 Gerrit-Owner: neels <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-Attention: fixeria <[email protected]> Gerrit-Comment-Date: Tue, 20 Aug 2024 22:30:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <[email protected]>
