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

[Alexander Turenko <alexander.turenko@tarantool.org>]

On Sat, Mar 07, 2020 at 10:02:28PM +0000, Nikita Pettik wrote:
> 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.)

If you want my personal opinion: pick up any way and go ahead. I was
asked by Kirill to answer how to extract helpers from a test, that is
why I'm here.

I still think that a fix of a certain property should be considered as a
bugfix, but it does not worth to spend time to discuss it.

WBR, Alexander Turenko.