Re: [Tarantool-patches] [PATCH v2] iproto: add an empty body to the unprepare response

[Nikita Pettik <korablev@tarantool.org>]

On 06 Mar 22:58, Chris Sosnin wrote:
> 
> > On 6 Mar 2020, at 20:27, Alexander Turenko <alexander.turenko@tarantool.org> wrote:
> > 
> > On Thu, Mar 05, 2020 at 08:41:35AM +0000, Nikita Pettik wrote:
> >> On 05 Mar 08:41, Kirill Yukhin wrote:
> >>> Hello,
> >>> 
> >>> On 03 мар 19:16, Chris Sosnin wrote:
> >>>> Absence of the body in the unprepare response forces users to perform
> >>>> additional checks to avoid errors. Adding an empty body fixes this problem.
> >>>> 
> >>>> Closes #4769
> >>>> ---
> >>>> branch: https://github.com/tarantool/tarantool/tree/ksosnin/gh-4769-unprepare-response-body
> >>>> issue: https://github.com/tarantool/tarantool/issues/4769
> >>>> 
> >>>> As Nikita suggested, I created box/iproto.test.lua, and basically
> >>>> inserted wrappers for requests testing from box-py for future usage.
> >>> 
> >>> Could you please rename the test to be not so generic?
> >>> Like box/gh-4769-iproto-unprep-body or whatever.
> >> 
> >> Kirill, this test is going to assemble all iproto-related tests
> >> which don't rely on net.box module. Setting up all preparations
> >> required for raw iproto communication results in duplicating ~30-40
> >> lines of code in each test file.
> > 
> > Technically there are two ways to extract helpers from a 'core =
> > tarantool' test:
> > 
> > * Add it to, say, test/box/box.lua and to _G.protected_globals.
> > * Add it to a separate Lua file in test/box/lua and to 'lua_libs' field
> >  in test/box/suite.ini. After this you can use `require` for this
> >  module in a test.
> 
> This also seems like a fine solution, if we are to stick to the SOP, I will do this.
> 
> However, I’m not sure whether this patch fixes a bug, it is stated in the code
> that there’s nothing to send in case of unprepare, perhaps it is a feature?

It's neither bug nor feature. I didn't realize that all other
requests have empty body (what is more, docs say that nop hasn't body
but in fact nop does have body tho) while was writing this code.
As it is stated in the issue, to unify raw statement processing it is
better to add empty body to unprepare response - just refactoring that
may simplify smb's life. That's it.

Anyway, I will introduce box/iproto.test.lua since stacked diag is
a feature. (It is so ironic that developers straggle from decision
which was taken solely by Kirill.)
 
> I will resend v3 if no one gives other proposals.
> Thank you for participating in discussion!
> 
> > 
> > So technically you're not blocked here. Both ways are available and
> > don't lead to much code duplication, but the process (SOP) requires to
> > add a test for a bug to a separate file. (Personally I still don't sure
> > it is good, but anyway.)
> > 
> > NB: 'receive', not 'recieve'. Very often typo.
> 
> Thanks for the catch here too, I fixed it for the future.
> 
> > 
> > WBR, Alexander Turenko.
>