From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp14.mail.ru (smtp14.mail.ru [94.100.181.95]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id C2730469719 for ; Fri, 6 Mar 2020 22:58:30 +0300 (MSK) From: Chris Sosnin Message-Id: <91892E2F-8CEF-4461-9AF5-512016633F8A@tarantool.org> Content-Type: multipart/alternative; boundary="Apple-Mail=_BF94F423-213F-48D5-A994-B5430886E242" Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.60.0.2.5\)) Date: Fri, 6 Mar 2020 22:58:27 +0300 In-Reply-To: <20200306172701.sziodfignjz2ix6a@tkn_work_nb> References: <20200303161649.62470-1-k.sosnin@tarantool.org> <20200305054101.6btvhiglj6olbijv@tarantool.org> <20200305084135.GC9655@tarantool.org> <20200306172701.sziodfignjz2ix6a@tkn_work_nb> Subject: Re: [Tarantool-patches] [PATCH v2] iproto: add an empty body to the unprepare response List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Turenko Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy --Apple-Mail=_BF94F423-213F-48D5-A994-B5430886E242 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On 6 Mar 2020, at 20:27, Alexander Turenko = wrote: >=20 > On Thu, Mar 05, 2020 at 08:41:35AM +0000, Nikita Pettik wrote: >> On 05 Mar 08:41, Kirill Yukhin wrote: >>> Hello, >>>=20 >>> On 03 =D0=BC=D0=B0=D1=80 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. >>>>=20 >>>> Closes #4769 >>>> --- >>>> branch: = https://github.com/tarantool/tarantool/tree/ksosnin/gh-4769-unprepare-resp= onse-body >>>> issue: https://github.com/tarantool/tarantool/issues/4769 >>>>=20 >>>> As Nikita suggested, I created box/iproto.test.lua, and basically >>>> inserted wrappers for requests testing from box-py for future = usage. >>>=20 >>> Could you please rename the test to be not so generic? >>> Like box/gh-4769-iproto-unprep-body or whatever. >>=20 >> 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. >=20 > Technically there are two ways to extract helpers from a 'core =3D > tarantool' test: >=20 > * 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=E2=80=99m not sure whether this patch fixes a bug, it is = stated in the code that there=E2=80=99s 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! >=20 > 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.) >=20 > NB: 'receive', not 'recieve'. Very often typo. Thanks for the catch here too, I fixed it for the future. >=20 > WBR, Alexander Turenko. --Apple-Mail=_BF94F423-213F-48D5-A994-B5430886E242 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8
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 =D0=BC=D0=B0=D1=80 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-unp= repare-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 =3D
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=E2=80=99m not = sure whether this patch fixes a bug, it is stated in the = code
that there=E2=80=99s 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.

= --Apple-Mail=_BF94F423-213F-48D5-A994-B5430886E242--