Page MenuHomePhabricator

Use lists instead of whitespace-separated strings in vyos.config
Open, Requires assessmentPublicBUG

Description

Using whitespace-separated strings for config paths in vyos.config is my original sin. It was done that way because all existing tools do it that way, including cli-shell-api and the Perl Vyatta::Config module. It's also a wrong way to do it.

Lists are much better for multiple reasons:

  • They can be safely concatenated without having to watch the whitespace (path + ["word", value] rather than path + ' word ' + value or format string)
  • They are easy to modify or retrieve elements from
  • Paths are logically lists of nodes, and whitespace-separated string is an unnecessary abstraction

Now that the old Perl code is rarely touched and 99% new code is in Python, the "histerical raisins" for doing it have calmed down and no longer apply. However, we have a bunch of scripts already using the whitespace-separated syntax.

The way out as I see it:

  1. Add dynamic dispatch to vyos.config to allow using either strings or lists for now
  2. Enforce through code review that new code uses lists
  3. Gradually adjust the old scripts to use the new approach

Details

Difficulty level
Hard (possibly days)
Version
1.3.0
Why the issue appeared?
Will be filled on close
Is it a breaking change?
Behavior change

Event Timeline

hagbard added a subscriber: hagbard.EditedTue, Dec 3, 10:27 PM

@dmbaturin

c.return_value('local-ip') &&  c.return_value(['local-ip'])

would work, shall I rewrite it to a single item list? conf.exists() does only accept s string as argument.