Inline..
—
Damjan
> On 12.05.2022., at 09:06, Andrew 👽 Yourtchenko <ayour...@gmail.com> wrote:
>
> Damjan,
>
> it is true we do not “guarantee” the behavior of the CLIs, but it is also
> true that a lot of people use them, that is why this warrants a bit more
> discussion and heads up.
Lot of people use cli, but not so much use packet-genrator or multiline
comments from startup.conf "unix exec”. Those who do sooner or later will ned
to spend few minutes to fix them, and I’m pretty sure earth will not stop
spinning for them.
>
> This issue was there since day 1
Yes, but looks like it starts annoying some people, and patches started showing
up in gerrit. Unfortunately those patches and just workarounds dealing with
single cli and I will prefer to have proper fix merged asap instead of merging
those workarounds.
> . What is driving the urgency of it ?
I didn’t say it is urgent, I said that it is better to get it fixed in 22.06
than not having it fixed in 22.06 and I elaborated why.
>
> --a
>
>> On 11 May 2022, at 14:29, Damjan Marion via lists.fd.io
>> <dmarion=me....@lists.fd.io> wrote:
>>
>>
>> I didn’t hear back from anybody, so let me elaborate this change a bit.
>>
>> It is clear that current behaviour is broken, as some CLI handlers which
>> work well in interactive mode simply eat rest of the content of exec file.
>> As result of that I can see people submitting patches to fix CLI handlers
>> they are interested in only, which i believe is wrong. This needs to be
>> fixed inside infra.
>>
>> Patch I submitted doesn’t change behaviour of CLIs which are written in
>> single line, but requires change in multiline CLIs which uses data wrapped
>> with {}. I am aware of 2 commands which do that:
>> - ‘comment' with multiline comments
>> - ‘packet-generator new’
>>
>> Funny thing about ‘packet-generator new’ is that typically needs to be
>> followed by “packet-generator enable” which also eats rest of the input
>> which makes usage of packet generator from exec script limited and
>> constrained. i.e. when i use packet generator typically i want to do some
>> show commands after and that is not possible today.
>>
>> I know very few people who use packet-genrator from exec script, they are
>> developers and I’m quite sure they should be able to address this issue
>> quickly.
>>
>> I fully disagree that we wait for next release with this change for
>> following reasons:
>> - we don’t guarantee CLI consistency like we do for APIs
>> - If this is right thing to do (and I didn’t hear anybody disagreeing), it
>> needs to happen sooner or later, so it is better to have this issue fixed
>> sooner that later
>> - reason for not merging patch late in development cycles is high risk of
>> bugs and that is not the case here. This patch is low risk from bugs
>> perspective, it just changes behaviour for very limited number of CLIs, and
>> there is more than enough time to document that before release is out.
>>
>> —
>> Damjan
>>
>>
>>
>>> On 09.05.2022., at 13:56, Damjan Marion via lists.fd.io
>>> <dmarion=me....@lists.fd.io> wrote:
>>>
>>>
>>> Hmm, not sure I understand concern about blast radius. I also replied to
>>> your comment in gerrit.
>>>
>>> —
>>> Damjan
>>>
>>>
>>>
>>>>> On 09.05.2022., at 07:31, Andrew 👽 Yourtchenko <ayour...@gmail.com> wrote:
>>>>
>>>> Damjan,
>>>>
>>>> I have left the comment on the change itself - in short, given its blast
>>>> radius, it needs to wait at least until 22.06 RC1 is done.
>>>>
>>>> --a
>>>>
>>>>> On 8 May 2022, at 19:39, Damjan Marion via lists.fd.io
>>>>> <dmarion=me....@lists.fd.io> wrote:
>>>>>
>>>>> Guys,
>>>>>
>>>>> I just submitted following patch which fixes long standing issue in how
>>>>> cli scripts are executed.
>>>>>
>>>>> https://gerrit.fd.io/r/c/vpp/+/36101
>>>>>
>>>>> Problem was that there was no way to execute CLIs which have optional
>>>>> arguments. I.e. “show version” and “show version verbose”.
>>>>> CLI parser was passing whole contents up to the EOF to each CLI handler,
>>>>> and because he eats all whitespaces there was no way to know if current
>>>>> unformat input points to the rest of the line or to the beginning of the
>>>>> new line.
>>>>>
>>>>> In this patch i changed that behaviour so CLI gets only one line of input.
>>>>>
>>>>> Also I changed unformat_input function so it recognises backslash before
>>>>> newline as way to pass multiline data to cli handler.
>>>>>
>>>>> As a result, there is no need for calling unformat_line in each cli
>>>>> handler, and still there is a way to specify multiline CLIs.
>>>>>
>>>>> i.e.
>>>>>
>>>>> show version \
>>>>> verbose
>>>>>
>>>>> or:
>>>>>
>>>>> packet-generator new { \
>>>>> name x \
>>>>> limit 5 \
>>>>> size 128-128 \
>>>>> interface local0 \
>>>>> node null-node \
>>>>> data { \
>>>>> incrementing 30 \
>>>>> } \
>>>>> }
>>>>>
>>>>> Hope nobody have issues with this change, but let me know if I’m wrong…
>>>>>
>>>>> —
>>>>> Damjan
>>>>>
>>>>>
>>>
>>>
>>>
>>
>>
>>
>>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#21402): https://lists.fd.io/g/vpp-dev/message/21402
Mute This Topic: https://lists.fd.io/mt/90974441/21656
Group Owner: vpp-dev+ow...@lists.fd.io
Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
