From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp49.i.mail.ru (smtp49.i.mail.ru [94.100.177.109]) (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 7F12F469719 for ; Sun, 8 Mar 2020 01:02:30 +0300 (MSK) Date: Sat, 7 Mar 2020 22:02:28 +0000 From: Nikita Pettik Message-ID: <20200307220228.GF12214@tarantool.org> References: <20200303161649.62470-1-k.sosnin@tarantool.org> <20200305054101.6btvhiglj6olbijv@tarantool.org> <20200305084135.GC9655@tarantool.org> <20200306172701.sziodfignjz2ix6a@tkn_work_nb> <91892E2F-8CEF-4461-9AF5-512016633F8A@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <91892E2F-8CEF-4461-9AF5-512016633F8A@tarantool.org> 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: Chris Sosnin Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy On 06 Mar 22:58, Chris Sosnin wrote: > > > On 6 Mar 2020, at 20:27, Alexander Turenko 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. >