On 5/25/22 16:32, Eelco Chaudron wrote:
>
>
> On 25 May 2022, at 16:10, Harry van Haaren wrote:
>
>> This commit reverts the name-change that was done (prio->info).
>> The change breaks a user visible ovs-appctl command, resulting in
>> breakage of tools/scripts/user-expectation outside of the OVS repo.
>>
>> This commit changes the documentation, command string, and unit tests
>> back to the expected "prio" string, as expected in OVS 2.17 and earlier.
>
> Can’t remember all the details, but I guess Ilya’s concern was that the
> command name does not reflect the actual output.
>
> So what about hiding the “subtable-lookup-prio-get” command, and making it
> such that it still works for backward compatibility?
> I think this can easily be done by adding the command with a NULL usage
> string.
>
> I guess all that needs to be added is another command, something like, but
> please test it:
>
> + unixctl_command_register("dpif-netdev/subtable-lookup-prio-get", NULL,
> + 0, 0, dpif_netdev_subtable_lookup_get,
> + NULL);
>
> Harry/Ilya what do you think?
The unlisted alias looks like a good middle ground to me.
The original patch is also missing the NEWS file update that
should be added, mentioning the re-name.
At the same time, I'm not sure if we actually need to maintain
backward compatibility here as this command can hardly be used
in automated scripts. The output format is changed significantly,
so any automated tools will have to be modified anyway.
In general, appctl APIs are supposed to be used mostly by humans,
especially "get/show/etc" ones as we're not maintaining the exact
output format for them. And if we're not maintaining the output
format, there is no much value in maintaining the command name.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
