[tarantool-patches] Re: [PATCH] Replace net.box usage with console in tarantoolctl eval

[Vladislav Shpilevoy]

Hello. Thanks for the fixes! See my comment below.

The patch LGTM now. Vova, please, make a second review.

>>> diff --git a/extra/dist/tarantoolctl.in b/extra/dist/tarantoolctl.in
>>> index 507ebe8bf..463d3e0a1 100755
>>> --- a/extra/dist/tarantoolctl.in
>>> +++ b/extra/dist/tarantoolctl.in
>>> @@ -647,13 +647,29 @@ local function stdin_isatty()
>>>   end
>>>     local function execute_remote(uri, code)
>>> -    local remote = netbox.connect(uri, {
>>> -        console = true, connect_timeout = TIMEOUT_INFINITY
>>> -    })
>>> -    if remote == nil then
>>> -        return nil
>>> +    local err, ret
>>> +    console.on_start(function(self)
>>> +        local cmd = string.format(
>>> +            "require('console').connect('%s', { connect_timeout = %s })",
>>> +            uri, TIMEOUT_INFINITY
>>> +        )
>>> +        err = self:eval(cmd)
>>
>> 1. Please, explain why do we connect in such complicated way? Why not just
>>
>>     console.connect(uri, {connect_timeout = TIMEOUT_INFINITY})
> I copied this connection method from function enter(), because console.connect()
> didn't work for me for some reason (probably did something wrong).

Console.connect works only when a console is started. See console.connect source.
Console.start creates internal console object and puts it into the fiber local
storage. Then this object is used by console.connect().

In 'tarantool>' interactive console you do not execute console.start because it
is started already, but in scripts you should start it explicitly, like that:

     console.start()
     console.on_start(do something with console and stop it)

> diff --git a/extra/dist/tarantoolctl.in b/extra/dist/tarantoolctl.in
> index 507ebe8bf..3b1c38585 100755
> --- a/extra/dist/tarantoolctl.in
> +++ b/extra/dist/tarantoolctl.in
> @@ -647,13 +647,19 @@ local function stdin_isatty()
>   end
> 
>   local function execute_remote(uri, code)
> -    local remote = netbox.connect(uri, {
> -        console = true, connect_timeout = TIMEOUT_INFINITY
> -    })
> -    if remote == nil then
> -        return nil
> -    end
> -    return true, remote:eval(code)
> +    local status, ret
> +    console.on_start(function(self)
> +        status, ret = pcall(console.connect, uri,
> +                            {connect_timeout = TIMEOUT_INFINITY})
> +        if status then
> +            status, ret = pcall(self.eval, self, code)
> +        end
> +        self.running = false
> +    end)
> +    console.on_client_disconnect(function(self) self.running = false end)
> +
> +    console.start()
> +    return status, ret
>   end