cli: restore default value for device interface create --bandwidth#3775
Conversation
PR #3077 dropped default_value from the bandwidth clap attribute on device interface create, which made --bandwidth mandatory in practice because the field is u64 (not Option<u64>). The PR description claimed --bandwidth was now optional, but only cir actually was. Restores the default to '0bps' to match cir, and adds a regression test that parses the CLI without --bandwidth.
Satisfies the CHANGELOG check on PR #3775.
|
I think we need to scope this out a bit more:
We could place a CLI check that prevents: |
|
@thijsvanemmerik @ben-malbeclabs If that sounds good to everyone, we can move forward with this change and I can send a follow-up PR with the validations Ben suggested. Then we can merge them in sequence. |
|
Sounds good Juan, thanks for picking up the follow-up. Happy to keep this PR as the minimal loopback regression fix. Ben, your scoping is right. The two checks sit naturally in different commands: require non-zero bandwidth for CYOA and DIA interfaces at |
|
New PR: #3778 |
juan-malbeclabs
left a comment
There was a problem hiding this comment.
LGTM -> #3778 bandwidth validation
Summary
--bandwidthondoublezero device interface createbecame mandatory after PR cli: display bandwidth/cir as human-readable strings, update help text #3077, even though that PR's description stated it was now optional.default_value = "0"from the clap attribute onbandwidth. Because the field isu64(notOption<u64>), clap then treats omission as a missing required argument. Onlyciractually became optional in that PR.default_value = "0bps"to match the existingcirattribute on the next line.0bpsis a valid input forvalidate_parse_bandwidth.Testing Verification
test_cli_device_interface_create_bandwidth_is_optionalthat usesclap::try_parse_fromwithout--bandwidthand asserts parsing succeeds withbandwidth == 0.MissingRequiredArgumentfor--bandwidth <BANDWIDTH>, and passes after the fix.doublezero_cliunit tests pass.