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

[Chris Sosnin <k.sosnin@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 2225 bytes --]


> 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?

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.


[-- Attachment #2: Type: text/html, Size: 14409 bytes --]