Page MenuHomePhabricator

Missing completion helper for "set system syslog host 192.0.2.1 facility all protocol"
Backport candidate, Requires assessmentPublicBUG

Description

Completion helper for "tcp" and "udp" is missing in command: set system syslog host 192.0.2.1 facility all protocol

Details

Difficulty level
Normal (likely a few hours)
Version
1.2.3
Why the issue appeared?
Will be filled on close
Is it a breaking change?
Perfectly compatible

Event Timeline

c-po created this task.Wed, Nov 27, 10:35 AM
pasik added a subscriber: pasik.Wed, Nov 27, 6:19 PM
hagbard claimed this task.Wed, Nov 27, 10:58 PM
hagbard changed the task status from Open to On hold.Wed, Nov 27, 11:09 PM

@c-po I used the 1.2.3 iso from the download portal and can't reproduce the issue. The help message was also already there since it was (re)implemented, could it be an issue with your installation?

c-po added a comment.EditedThu, Nov 28, 4:45 AM

Nope, I was talking about completionHelper, which is not implemented - same as constraint nodes for udp/tcp regex.

https://github.com/vyos/vyos-1x/blob/crux/interface-definitions/syslog.xml#L286-L298

There is a difference in valueHelp and completionHelp. valueHelp shows whats possible and completionHelp will conplete the CLI command once you hit TAB.

Example: https://github.com/vyos/vyos-1x/blob/current/interface-definitions/interfaces-wireless.xml#L164-L183

hagbard changed the task status from On hold to In progress.EditedThu, Nov 28, 4:16 PM

Gotcha, I start implement it for the rolling release for now. Since it affects multiple other nodes as well, I'll do it for all of them equally. aka facility has no constraint and completion help too. There are a few more I've found, so I crawl through all of them and enhance it a bit.

hagbard changed Difficulty level from Easy (less than an hour) to Normal (likely a few hours).Thu, Nov 28, 4:30 PM
c-po added a comment.Thu, Nov 28, 7:40 PM

Thanks, should be backported to crux then as I found it there

hagbard changed the task status from Needs testing to Backport candidate.Thu, Nov 28, 11:15 PM
hagbard added a comment.EditedWed, Dec 4, 11:45 PM

Will interfere with T1845, while I think T1845 should be the backport candidate. Facility is a tag node, protocol has been moved on level up and is host specific. Having the availability to set different port per facility doesn't really make sense.

c-po added a comment.Thu, Dec 5, 7:21 AM

Wha cant the change which adds the completion helper be backported? Git best practice is you should group changes per commit. But in this commit you add the completion helper and also other regexes (BAD!)

You can via https://github.com/vyos/vyos-1x/commit/dad110ce666edae42ac18c59a800bda503589f27, which just sets the completion help.
For T1845, yes it solves the issue with setting address:port _and_ moves protocol up from facility to host. Do you want me to revert and do 2 commits, which requires then 2 migrations, once for address:port and one to solve the logical issue with protocol. Right now you can set a different protocol for different facilities for the same host.