[tarantool-patches] Re: [PATCH] tarantoolctl: remove metatable assumptions in start()
Roman Khabibov
roman.habibov at tarantool.org
Sat Apr 6 02:26:20 MSK 2019
Hello! Thanks for review.
> On Apr 1, 2019, at 8:43 AM, Alexander Turenko <alexander.turenko at tarantool.org> wrote:
>
> Commented in the issue, cited here:
>
>> @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.
>>
>> Re config module errors: I don't know whether you use it correctly. If
>> so, file another issue to this module.
>>
>> https://github.com/tarantool/tarantool/issues/3953#issuecomment-478439448
>
> I propose to consider this issue as a problem with an error message.
>
> WBR, Alexander Turenko.
>
> 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.
>>
>> Closes #3953
>> ---
>> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3953-tctl
>> Issue: https://github.com/tarantool/tarantool/issues/3953
>>
>> extra/dist/tarantoolctl.in | 7 -------
>> test/app-tap/tarantoolctl.test.lua | 4 +++-
>> 2 files changed, 3 insertions(+), 8 deletions(-)
>>
>> 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 = getmetatable(box.cfg).__call
>> - getmetatable(box.cfg).__call = function(old_cfg, cfg)
>> - if old_cfg.pid_file ~= nil and cfg ~= nil and cfg.pid_file ~= nil then
>> - cfg.pid_file = old_cfg.pid_file
>
> So if a user lean on that value (s)he now cannot?
>
>> - end
>> - old_call(old_cfg, cfg)
>
> So if a user set a metatable on box.cfg() with __call method, it will
> not more called?
>
>> - end
>> return 0
>> end
>>
>> 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 = fio.tempdir()
>> local code = [[ box.cfg{memtx_memory = 104857600} ]]
>> create_script(dir, 'script.lua', code)
>> + create_script(dir, 'init.lua', [[ print('Hi!') ]])
>>
>> local status, err = 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…")
Reworked.
+ local m_table = getmetatable(box.cfg)
+ if m_table == nil then
+ error("Plaese, call box.cfg{} within your instance")
+ end
+ local old_call = 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.
>
>> 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")
>> --
>> 2.20.1 (Apple Git-117)
check_ok(test_i, dir, 'status', 'script', 0, nil, "is running”) - I think it’s OK. Instance ’script’ have already been started, so ’tarantoolctl status script’ returns 0.
commit 4cdf8e79bb530288098fb30e5014498fc0ad4262
Author: Roman Khabibov <roman.habibov at tarantool.org>
Date: Mon Mar 25 20:52:05 2019 +0300
tarantoolctl: error when box.cfg didn't called
There was assumpted uninitialized metamethods of box.cfg{} if user
did't call it in his instance.
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 = getmetatable(box.cfg).__call
+ local m_table = getmetatable(box.cfg)
+ if m_table == nil then
+ error("Plaese, call box.cfg{} within your instance")
+ end
+ local old_call = m_table.__call
getmetatable(box.cfg).__call = function(old_cfg, cfg)
if old_cfg.pid_file ~= nil and cfg ~= nil and cfg.pid_file ~= nil then
cfg.pid_file = 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 = fio.tempdir()
local code = [[ box.cfg{memtx_memory = 104857600} ]]
create_script(dir, 'script.lua', code)
+ create_script(dir, 'init.lua', [[ print('Hi!') ]])
local status, err = 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")
More information about the Tarantool-patches
mailing list