From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id CB41D2A088 for ; Wed, 19 Sep 2018 09:38:33 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id o_b4XfqqYFQK for ; Wed, 19 Sep 2018 09:38:33 -0400 (EDT) Received: from smtp56.i.mail.ru (smtp56.i.mail.ru [217.69.128.36]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 102682A074 for ; Wed, 19 Sep 2018 09:38:32 -0400 (EDT) Date: Wed, 19 Sep 2018 16:38:31 +0300 From: Alexander Turenko Subject: [tarantool-patches] Re: [PATCH] box: add a method to check if transaction is open Message-ID: <20180919133830.bvtnqh3ftbdukobh@tkn_work_nb> References: <1536282019-17978-1-git-send-email-roman.habibov1@yandex.ru> <41907181536625411@iva8-8d7a47df0521.qloud-c.yandex.net> <41826271536625517@iva7-bd007c44f58e.qloud-c.yandex.net> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: Roman Khabibov Cc: tarantool-patches@freelists.org, Kirill Shcherbatov Hi, Roman! Sorry for the late response. Please, consider comments below. WBR, Alexander Turenko. On Tue, Sep 11, 2018 at 02:51:19PM +0300, Kirill Shcherbatov wrote: > Now LGTM. Sasha, please, take a look. > > >> commit 14fa90d195dc84df21a41c3172d2f0a2e0ca68e2 > >> Author: Roman Khabibov > >> Date: Fri Sep 7 02:54:28 2018 +0300 > >> > >>     box: add a method to check if txn is open > >> > >>     Closes: #3518. > >> > >> +box.is_in_txn = function() > >> + return { builtin.box_txn() } > >> +end > >> + Why do you return a table with boolean, but not boolean? > >> diff --git a/test/engine/savepoint.result b/test/engine/savepoint.result > >> index a62a2e1..00d281b 100644 > >> --- a/test/engine/savepoint.result > >> +++ b/test/engine/savepoint.result > >> @@ -14,7 +14,7 @@ s1 = box.savepoint() > >>  ... > >>  box.rollback_to_savepoint(s1) > >>  --- > >> -- error: 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)' > >> +- error: 'builtin/box/schema.lua:346: Usage: box.rollback_to_savepoint(savepoint)' > >>  ... > >>  box.begin() s1 = box.savepoint() > >>  --- > >> @@ -323,27 +323,27 @@ test_run:cmd("setopt delimiter ''"); > >>  ok1, errmsg1 > >>  --- > >>  - false > >> -- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)' > >> +- 'builtin/box/schema.lua:346: Usage: box.rollback_to_savepoint(savepoint)' > >>  ... > >>  ok2, errmsg2 > >>  --- > >>  - false > >> -- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)' > >> +- 'builtin/box/schema.lua:346: Usage: box.rollback_to_savepoint(savepoint)' > >>  ... > >>  ok3, errmsg3 > >>  --- > >>  - false > >> -- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)' > >> +- 'builtin/box/schema.lua:346: Usage: box.rollback_to_savepoint(savepoint)' > >>  ... > >>  ok4, errmsg4 > >>  --- > >>  - false > >> -- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)' > >> +- 'builtin/box/schema.lua:346: Usage: box.rollback_to_savepoint(savepoint)' > >>  ... > >>  ok5, errmsg5 > >>  --- > >>  - false > >> -- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)' > >> +- 'builtin/box/schema.lua:346: Usage: box.rollback_to_savepoint(savepoint)' > >>  ... > >>  s:select{} > >>  --- > > It is better to use output filtering feature of test-run to avoid such redundant diffs. Consider test/app/socket.test.lua: ``` 12 env = require('test_run') 13 test_run = env.new() 14 test_run:cmd("push filter '(error: .builtin/.*[.]lua):[0-9]+' to '\\1'") ``` It is not a blocker for that patch, but if you intend to include it, do it in the separate commit before 'add a method to check if txn is open'.