—
Damjan
> On 12.05.2022., at 15:12, Andrew 👽 Yourtchenko <ayour...@gmail.com> wrote:
>
> Inline
>
>> On 12 May 2022, at 14:21, Damjan Marion <dmar...@me.com> wrote:
>>
>> 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.
>
> Every time a change is forced on someone, they might have to interrupt what
> they were doing, in order to deal with that change. The earth doesn’t stop
> spinning, as you say, but it’s much nicer to have a little heads up on these
> things, like an order of a couple of weeks or so.
> Maybe in this case there is no one who is affected in this case. But I do not
> know. And when I do not know I try to err on a side of caution.
Exact purpose of this thread was to give heads up, and opportunity to somebody
who is really impacted to say so.
>
>>
>>> 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.
>
> As you rightly said, we do not guarantee the stability of the CLIs, I removed
> the -2. Please accept my apologies, it probably should’ve been a -1 in the
> first place.
Thanks!
>
> —a
>
>>
>>> --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 (#21405): https://lists.fd.io/g/vpp-dev/message/21405
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]
-=-=-=-=-=-=-=-=-=-=-=-
