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 5079D2BE43 for ; Fri, 5 Apr 2019 19:26:24 -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 F9Uujv8IfFP7 for ; Fri, 5 Apr 2019 19:26:24 -0400 (EDT) Received: from smtp32.i.mail.ru (smtp32.i.mail.ru [94.100.177.92]) (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 812162B042 for ; Fri, 5 Apr 2019 19:26:23 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: [tarantool-patches] Re: [PATCH] tarantoolctl: remove metatable assumptions in start() From: Roman Khabibov In-Reply-To: <20190401054304.kofvdg3uz6fzwl5d@tkn_work_nb> Date: Sat, 6 Apr 2019 02:26:20 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <20190327134014.56676-1-roman.habibov@tarantool.org> <20190401054304.kofvdg3uz6fzwl5d@tkn_work_nb> 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: tarantool-patches@freelists.org Cc: Alexander Turenko Hello! Thanks for review. > On Apr 1, 2019, at 8:43 AM, Alexander Turenko = wrote: >=20 > Commented in the issue, cited here: >=20 >> @opomuc @olegrok I don't know what are you trying to do here. It = seems >> you just try to start an instance w/o box.cfg() call inside. I don't >> see a reason to support this case. However more meaningful error >> message from tarantoolctl start in the case would helpful. >>=20 >> Re config module errors: I don't know whether you use it correctly. = If >> so, file another issue to this module. >>=20 >> = https://github.com/tarantool/tarantool/issues/3953#issuecomment-478439448 >=20 > I propose to consider this issue as a problem with an error message. >=20 > WBR, Alexander Turenko. >=20 > On Wed, Mar 27, 2019 at 04:40:14PM +0300, Roman Khabibov wrote: >> box.cfg{} metatables are uninitialized when start() called >> before box.cfg{}. It led to unexpected error. >>=20 >> Closes #3953 >> --- >> Branch: = https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3953-tctl >> Issue: https://github.com/tarantool/tarantool/issues/3953 >>=20 >> extra/dist/tarantoolctl.in | 7 ------- >> test/app-tap/tarantoolctl.test.lua | 4 +++- >> 2 files changed, 3 insertions(+), 8 deletions(-) >>=20 >> diff --git a/extra/dist/tarantoolctl.in b/extra/dist/tarantoolctl.in >> index 47fcf895f..91747e3ba 100755 >> --- a/extra/dist/tarantoolctl.in >> +++ b/extra/dist/tarantoolctl.in >> @@ -547,13 +547,6 @@ local function start() >> end >> os.exit(1) >> end >> - local old_call =3D getmetatable(box.cfg).__call >> - getmetatable(box.cfg).__call =3D function(old_cfg, cfg) >> - if old_cfg.pid_file ~=3D nil and cfg ~=3D nil and = cfg.pid_file ~=3D nil then >> - cfg.pid_file =3D old_cfg.pid_file >=20 > So if a user lean on that value (s)he now cannot? >=20 >> - end >> - old_call(old_cfg, cfg) >=20 > So if a user set a metatable on box.cfg() with __call method, it will > not more called? >=20 >> - end >> return 0 >> end >>=20 >> diff --git a/test/app-tap/tarantoolctl.test.lua = b/test/app-tap/tarantoolctl.test.lua >> index db046e03f..1755fcc66 100755 >> --- a/test/app-tap/tarantoolctl.test.lua >> +++ b/test/app-tap/tarantoolctl.test.lua >> @@ -165,10 +165,12 @@ do >> local dir =3D fio.tempdir() >> local code =3D [[ box.cfg{memtx_memory =3D 104857600} ]] >> create_script(dir, 'script.lua', code) >> + create_script(dir, 'init.lua', [[ print('Hi!') ]]) >>=20 >> local status, err =3D pcall(function() >> test:test("basic test", function(test_i) >> - test_i:plan(16) >> + test_i:plan(18) >> + check_ok(test_i, dir, 'start', 'init.lua', 0, nil, = "Starting instance init=E2=80=A6") Reworked. + local m_table =3D getmetatable(box.cfg) + if m_table =3D=3D nil then + error("Plaese, call box.cfg{} within your instance") + end + local old_call =3D m_table.__call > So we're 'successfully' started an instance, it executes a script and > then 'tarantoolctl status' and 'tarantoolctl enter' don't show it as > running. It does not look as a right behaviour. >=20 >> check_ok(test_i, dir, 'start', 'script', 0, nil, = "Starting instance") >> tctl_wait_start(dir, 'script') >> check_ok(test_i, dir, 'status', 'script', 0, nil, "is = running") >> --=20 >> 2.20.1 (Apple Git-117) check_ok(test_i, dir, 'status', 'script', 0, nil, "is running=E2=80=9D) = - I think it=E2=80=99s OK. Instance =E2=80=99script=E2=80=99 have = already been started, so =E2=80=99tarantoolctl status script=E2=80=99 = returns 0. commit 4cdf8e79bb530288098fb30e5014498fc0ad4262 Author: Roman Khabibov Date: Mon Mar 25 20:52:05 2019 +0300 tarantoolctl: error when box.cfg didn't called =20 There was assumpted uninitialized metamethods of box.cfg{} if user did't call it in his instance. =20 Closes #3953 diff --git a/extra/dist/tarantoolctl.in b/extra/dist/tarantoolctl.in index 47fcf895f..396fa7957 100755 --- a/extra/dist/tarantoolctl.in +++ b/extra/dist/tarantoolctl.in @@ -547,7 +547,11 @@ local function start() end os.exit(1) end - local old_call =3D getmetatable(box.cfg).__call + local m_table =3D getmetatable(box.cfg) + if m_table =3D=3D nil then + error("Plaese, call box.cfg{} within your instance") + end + local old_call =3D m_table.__call getmetatable(box.cfg).__call =3D function(old_cfg, cfg) if old_cfg.pid_file ~=3D nil and cfg ~=3D nil and cfg.pid_file = ~=3D nil then cfg.pid_file =3D old_cfg.pid_file diff --git a/test/app-tap/tarantoolctl.test.lua = b/test/app-tap/tarantoolctl.test.lua index db046e03f..428b074e2 100755 --- a/test/app-tap/tarantoolctl.test.lua +++ b/test/app-tap/tarantoolctl.test.lua @@ -165,10 +165,13 @@ do local dir =3D fio.tempdir() local code =3D [[ box.cfg{memtx_memory =3D 104857600} ]] create_script(dir, 'script.lua', code) + create_script(dir, 'init.lua', [[ print('Hi!') ]]) =20 local status, err =3D pcall(function() test:test("basic test", function(test_i) - test_i:plan(16) + test_i:plan(18) + check_ok(test_i, dir, 'start', 'init.lua', 1, nil, = "Starting instance init...", + "Plaese, call box.cfg{} within your instance") check_ok(test_i, dir, 'start', 'script', 0, nil, "Starting = instance") tctl_wait_start(dir, 'script') check_ok(test_i, dir, 'status', 'script', 0, nil, "is = running")