[tarantool-patches] [PATCH v4 0/5] box/lua/console: Add initial support for lua output format

[tarantool-patches] [PATCH v4 0/5] box/lua/console: Add initial support for lua output format

[Cyrill Gorcunov]

Hi, here is an updated series for lua console support. It is in devel state
still since we need to enhance output support for remote connections. By
now I limit this mode for local sessions only.

Main purpose of this series is to not break anything existing thus requires
a deep testing.

TODO:
 - extend exchange protocol with remote nodes to pass current output
   mode and get ack/nack to process
 - add compat layer for old remote server which do not have lua output
   support at all and will provide us yaml output regarding of anything
 - rework "help" and turorial output since in lua mode they are simply
   unreadable
 - extend test engine so that new tests would use lua output by default
   (should we?)

---
The following changes since commit e5e23ce2788ccfb6223a92f668483d7fb91352ca:

  test: update test-run (2019-07-22 11:28:58 +0300)

are available in the Git repository at:

  https://github.com/cyrillos/tarantool.git console-repl-serpent-4

for you to fetch changes up to eb53b99b6e9aa6534544ccadb6dad0ffaa9b1ab1:

  box/lua/console: Limit lua output for local sessions only (2019-07-24 01:27:50 +0300)

----------------------------------------------------------------
Cyrill Gorcunov (5):
      third_party/serpent: Add serpent repo
      box/lua/console: Add support for lua output format
      box/lua/console: Don't serialize function body
      box/lua/console: Provide output_default function to setup default output
      box/lua/console: Limit lua output for local sessions only

 .gitmodules             |   3 +++
 src/box/CMakeLists.txt  |   1 +
 src/box/lua/console.c   |  26 ++++++++++++++++++++
 src/box/lua/console.lua | 149 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/lua/help_en_US.lua  |   1 +
 test/box/admin.result   |   1 +
 third_party/serpent     |   1 +
 7 files changed, 181 insertions(+), 1 deletion(-)
 create mode 160000 third_party/serpent

[tarantool-patches] [PATCH 1/5] third_party/serpent: Add serpent repo

[Cyrill Gorcunov]

We will use it for Lua console output format serialization

Part-of #3834
---
 .gitmodules         | 3 +++
 third_party/serpent | 1 +
 2 files changed, 4 insertions(+)
 create mode 160000 third_party/serpent

diff --git a/.gitmodules b/.gitmodules
index 8cce59436..2db068148 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -34,3 +34,6 @@
 [submodule "third_party/decNumber"]
 	path = third_party/decNumber
 	url = https://github.com/tarantool/decNumber.git
+[submodule "third_party/serpent"]
+	path = third_party/serpent
+	url = git@github.com:tarantool/serpent.git
diff --git a/third_party/serpent b/third_party/serpent
new file mode 160000
index 000000000..879580fb2
--- /dev/null
+++ b/third_party/serpent
@@ -0,0 +1 @@
+Subproject commit 879580fb21933f63eb23ece7d60ba2349a8d2848
-- 
2.20.1

[tarantool-patches] Re: [PATCH 1/5] third_party/serpent: Add serpent repo

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/07/24 01:35]:
> We will use it for Lua console output format serialization

One more thing, for the future: all patches which add new features
should have a plea to the documentation bot.

I added one for your changes.


-- 
Konstantin Osipov, Moscow, Russia

[tarantool-patches] Re: [PATCH 1/5] third_party/serpent: Add serpent repo

[Cyrill Gorcunov]

On Wed, Jul 24, 2019 at 08:02:02PM +0300, Konstantin Osipov wrote:
> 
> One more thing, for the future: all patches which add new features
> should have a plea to the documentation bot.
> 
> I added one for your changes.

Thank you! I know about docs simply:
 - didn't know how to address particular page (since this
   changes should be in console module of documenation, right?)
 - and I thought that I should rathe make a final doc once I
   finish console development

[tarantool-patches] [PATCH 2/5] box/lua/console: Add support for lua output format

[Cyrill Gorcunov]

Historically we use YAML format to print results of operation to
a console. Moreover our test engine is aiming YAML as a primary format
to compare results of test runs. Still we need an ability to print
results in a different fasion, in particular one may need to use
the console in a REPL way so that the results would be copied and
pased back to further processing.

For this sake we introduce that named "output" command which allows
to specify which exactly output format to use. Currently only yaml
and lua formats are supported.

To specify lua output format type

 | tarantool> \set output lua

in the console. lua mode supports line oriented output (default) or
block mode.

For example

 | tarantool> a={1,2,3}
 | tarantool> a
 | ---
 | - - 1
 |   - 2
 |   - 3
 | ...
 | tarantool> \set output lua
 | true
 | tarantool> a
 | {1, 2, 3}
 | tarantool> \set output lua,block
 | true
 | tarantool> a
 | {
 |   1,
 |   2,
 |   3
 | }

By default YAML output format is kept for now, simply to not
break the test engine. The output is bound to a session, thus every
new session should setup own conversion if needed.

Since serializing lua data is not a trivial task we use "serpent"
third party module to convert data.

Part-of #3834
---
 src/box/CMakeLists.txt  |   1 +
 src/box/lua/console.c   |  26 ++++++++++
 src/box/lua/console.lua | 102 +++++++++++++++++++++++++++++++++++++++-
 src/lua/help_en_US.lua  |   1 +
 test/box/admin.result   |   1 +
 5 files changed, 130 insertions(+), 1 deletion(-)

diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index 481842a39..4b037094a 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -11,6 +11,7 @@ lua_source(lua_sources lua/feedback_daemon.lua)
 lua_source(lua_sources lua/net_box.lua)
 lua_source(lua_sources lua/upgrade.lua)
 lua_source(lua_sources lua/console.lua)
+lua_source(lua_sources ../../third_party/serpent/src/serpent.lua)
 lua_source(lua_sources lua/xlog.lua)
 lua_source(lua_sources lua/key_def.lua)
 lua_source(lua_sources lua/merger.lua)
diff --git a/src/box/lua/console.c b/src/box/lua/console.c
index cefe5d863..57e7e7f4f 100644
--- a/src/box/lua/console.c
+++ b/src/box/lua/console.c
@@ -46,6 +46,8 @@
 #include <stdlib.h>
 #include <ctype.h>
 
+extern char serpent_lua[];
+
 static struct luaL_serializer *luaL_yaml_default = NULL;
 
 /*
@@ -431,6 +433,24 @@ console_session_push(struct session *session, uint64_t sync, struct port *port)
 				     TIMEOUT_INFINITY);
 }
 
+static void
+lua_serpent_init(struct lua_State *L)
+{
+	static const char modname[] = "serpent";
+	const char *modfile;
+
+	lua_getfield(L, LUA_REGISTRYINDEX, "_LOADED");
+	modfile = lua_pushfstring(L, "@builtin/%s.lua", modname);
+	if (luaL_loadbuffer(L, serpent_lua, strlen(serpent_lua), modfile)) {
+		panic("Error loading Lua module %s...: %s",
+		      modname, lua_tostring(L, -1));
+	}
+
+	lua_call(L, 0, 1);
+	lua_setfield(L, -3, modname);  /* _LOADED[modname] = new table */
+	lua_pop(L, 2);
+}
+
 void
 tarantool_lua_console_init(struct lua_State *L)
 {
@@ -471,6 +491,12 @@ tarantool_lua_console_init(struct lua_State *L)
 	};
 	session_vtab_registry[SESSION_TYPE_CONSOLE] = console_session_vtab;
 	session_vtab_registry[SESSION_TYPE_REPL] = console_session_vtab;
+
+	/*
+	 * Register serpent serializer as we
+	 * need it inside console REPL mode.
+	 */
+	lua_serpent_init(L);
 }
 
 /*
diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index f922f0320..479046f80 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -1,5 +1,6 @@
 -- console.lua -- internal file
 
+local serpent = require('serpent')
 local internal = require('console')
 local session_internal = require('box.internal.session')
 local fiber = require('fiber')
@@ -13,7 +14,9 @@ local net_box = require('net.box')
 local YAML_TERM = '\n...\n'
 local PUSH_TAG_HANDLE = '!push!'
 
-local function format(status, ...)
+local output_handlers = { }
+
+output_handlers["yaml"] = function(status, opts, ...)
     local err
     if status then
         -- serializer can raise an exception
@@ -33,6 +36,92 @@ local function format(status, ...)
     return internal.format({ error = err })
 end
 
+output_handlers["lua"] = function(status, opts, ...)
+    local data = ...
+    --
+    -- Don't print nil if there is no data
+    if data == nil then
+        return ""
+    end
+    --
+    -- Map internals symbols which serpent don't know
+    -- about into known representation.
+    local map_symbols = function(tag, head, body, tail, level)
+        local symbols = {
+            ['"cdata<void %*>: NULL"']  = 'box.NULL'
+        }
+        for k,v in pairs(symbols) do
+            body = body:gsub(k, v)
+        end
+        return tag..head..body..tail
+    end
+    local serpent_opts = {
+        custom  = map_symbols,
+        comment = false,
+    }
+    if opts == "block" then
+        return serpent.block(..., serpent_opts)
+    end
+    return serpent.line(..., serpent_opts)
+end
+
+local function output_verify_opts(fmt, opts)
+    if opts == nil then
+        return true, nil
+    end
+    if fmt == "lua" then
+        if opts ~= "line" and opts ~= "block" then
+            local msg = 'Wrong option "%s", expecting: line or block.'
+            return false, msg:format(opts)
+        end
+    end
+    return true, nil
+end
+
+local function output_parse(value)
+    local fmt, opts
+    if value:match("([^,]+),([^,]+)") ~= nil then
+        fmt, opts = value:match("([^,]+),([^,]+)")
+    else
+        fmt = value
+    end
+    for k, _ in pairs(output_handlers) do
+        if k == fmt then
+            local status, err = output_verify_opts(fmt, opts)
+            if status ~= true then
+                return false, err
+            end
+            return true, nil, fmt, opts
+        end
+    end
+    local msg = 'Invalid format "%s", supported languages: lua and yaml.'
+    return false, msg:format(value)
+end
+
+local function output_save(fmt, opts)
+    --
+    -- Output format descriptors are saved per
+    -- session thus each console may specify
+    -- own mode.
+    box.session.storage.console_output = {
+        ["fmt"] = fmt, ["opts"] = opts
+    }
+end
+
+local function format(status, ...)
+    local d = box.session.storage.console_output
+    --
+    -- If there was no assignment yet provide
+    -- a default value; for now it is YAML
+    -- for backward compatibility sake (don't
+    -- forget about test results which are in
+    -- YAML format).
+    if d == nil then
+        d = { ["fmt"] = "yaml", ["opts"] = nil }
+    end
+    return output_handlers[d["fmt"]](status, d["opts"], ...)
+end
+
 --
 -- Set delimiter
 --
@@ -71,11 +160,22 @@ local function set_language(storage, value)
     return true
 end
 
+local function set_output(storage, value)
+    local status, err, fmt, opts = output_parse(value)
+    if status ~= true then
+        return error(err)
+    end
+    output_save(fmt, opts)
+    return true
+end
+
 local function set_param(storage, func, param, value)
     local params = {
         language = set_language,
         lang = set_language,
         l = set_language,
+        output = set_output,
+        o = set_output,
         delimiter = set_delimiter,
         delim = set_delimiter,
         d = set_delimiter
diff --git a/src/lua/help_en_US.lua b/src/lua/help_en_US.lua
index d37b49bf7..4f0f7d65b 100644
--- a/src/lua/help_en_US.lua
+++ b/src/lua/help_en_US.lua
@@ -8,6 +8,7 @@ To start the interactive Tarantool tutorial, type 'tutorial()' here.
 Available backslash commands:
 
   \set language <language>   -- set language (lua or sql)
+  \set output <format>       -- set output format (lua[,line|block] or yaml)
   \set delimiter <delimiter> -- set expression delimiter
   \help                      -- show this screen
   \quit                      -- quit interactive console
diff --git a/test/box/admin.result b/test/box/admin.result
index 906e01e8d..0c137e371 100644
--- a/test/box/admin.result
+++ b/test/box/admin.result
@@ -20,6 +20,7 @@ help()
     Available backslash commands:
 
       \set language <language>   -- set language (lua or sql)
+      \set output <format>       -- set output format (lua[,line|block] or yaml)
       \set delimiter <delimiter> -- set expression delimiter
       \help                      -- show this screen
       \quit                      -- quit interactive console
-- 
2.20.1

[tarantool-patches] [PATCH 3/5] box/lua/console: Don't serialize function body

[Cyrill Gorcunov]

Without this option serpent tries to encode function body,
making output a plain mess. So just like for yaml output
where functions are represented as

 | "completion_handler: 'function: 0x4074c910'

for lua output we will have

 | completion_handler = function() --[[..skipped..]] end,

Part-of #3834
---
 src/box/lua/console.lua | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index 479046f80..7120bc8e6 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -58,6 +58,7 @@ output_handlers["lua"] = function(status, opts, ...)
     local serpent_opts = {
         custom  = map_symbols,
         comment = false,
+        nocode = true,
     }
     if opts == "block" then
         return serpent.block(..., serpent_opts)
-- 
2.20.1

[tarantool-patches] [PATCH 4/5] box/lua/console: Provide output_default function to setup default output

[Cyrill Gorcunov]

A user might need to use lua output as default serializer so instead
of requiring him setting up the output every new session that named
default output mode may be used instead with help of command

 > require('console').set_default_output("lua")

Also we provide

 > require('console').get_default_output("lua")

to obtain current setting.

Part-of #3834
---
 src/box/lua/console.lua | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index 7120bc8e6..db57a6d80 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -14,6 +14,10 @@ local net_box = require('net.box')
 local YAML_TERM = '\n...\n'
 local PUSH_TAG_HANDLE = '!push!'
 
+--
+-- Default output handler set to YAML for backward
+-- compatibility reason.
+local ouput_default_handler = { ["fmt"] = "yaml", ["opts"] = nil }
 local output_handlers = { }
 
 output_handlers["yaml"] = function(status, opts, ...)
@@ -99,6 +103,22 @@ local function output_parse(value)
     return false, msg:format(value)
 end
 
+local function set_default_output(value)
+    if value == nil then
+        error("Nil output value passed")
+    end
+    local status, err, fmt, opts = output_parse(value)
+    if status ~= true then
+        error(err)
+    end
+    ouput_default_handler["fmt"] = fmt
+    ouput_default_handler["opts"] = opts
+end
+
+local function get_default_output(value)
+        return ouput_default_handler
+end
+
 local function output_save(fmt, opts)
     --
     -- Output format descriptors are saved per
@@ -111,14 +131,8 @@ end
 
 local function format(status, ...)
     local d = box.session.storage.console_output
-    --
-    -- If there was no assignment yet provide
-    -- a default value; for now it is YAML
-    -- for backward compatibility sake (don't
-    -- forget about test results which are in
-    -- YAML format).
     if d == nil then
-        d = { ["fmt"] = "yaml", ["opts"] = nil }
+        d = ouput_default_handler
     end
     return output_handlers[d["fmt"]](status, d["opts"], ...)
 end
@@ -674,6 +688,8 @@ package.loaded['console'] = {
     start = start;
     eval = eval;
     delimiter = delimiter;
+    set_default_output = set_default_output;
+    get_default_output = get_default_output;
     ac = ac;
     connect = connect;
     listen = listen;
-- 
2.20.1

[tarantool-patches] Re: [PATCH 4/5] box/lua/console: Provide output_default function to setup default output

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/07/24 01:35]:
> A user might need to use lua output as default serializer so instead
> of requiring him setting up the output every new session that named
> default output mode may be used instead with help of command
> 
>  > require('console').set_default_output("lua")
> 
> Also we provide
> 
>  > require('console').get_default_output("lua")
> 
> to obtain current setting.
> 
> Part-of #3834

there is a bug in this patch - it changes the output of an already
existing console. It should not happen, only new consoles should
be affected.

Please submit a fix.


-- 
Konstantin Osipov, Moscow, Russia

[tarantool-patches] Re: [PATCH 4/5] box/lua/console: Provide output_default function to setup default output

[Konstantin Osipov]

* Konstantin Osipov <kostja@tarantool.org> [19/07/24 20:10]:
> * Cyrill Gorcunov <gorcunov@gmail.com> [19/07/24 01:35]:
> > A user might need to use lua output as default serializer so instead
> > of requiring him setting up the output every new session that named
> > default output mode may be used instead with help of command
> > 
> >  > require('console').set_default_output("lua")
> > 
> > Also we provide
> > 
> >  > require('console').get_default_output("lua")
> > 
> > to obtain current setting.
> > 
> > Part-of #3834
> 
> there is a bug in this patch - it changes the output of an already
> existing console. It should not happen, only new consoles should
> be affected.
> 
> Please submit a fix.

The fix is to table.deepcopy() d from default.


-- 
Konstantin Osipov, Moscow, Russia

[tarantool-patches] Re: [PATCH 4/5] box/lua/console: Provide output_default function to setup default output

[Cyrill Gorcunov]

On Wed, Jul 24, 2019 at 08:11:41PM +0300, Konstantin Osipov wrote:
> > 
> > there is a bug in this patch - it changes the output of an already
> > existing console. It should not happen, only new consoles should
> > be affected.
> > 
> > Please submit a fix.
> 
> The fix is to table.deepcopy() d from default.

Thank you! Will do.

[tarantool-patches] [PATCH 5/5] box/lua/console: Limit lua output for local sessions only

[Cyrill Gorcunov]

To make full support of lua mode on remote connection we
need pass this option to remote server. Also we need to
provide a compatibility layer for servers from previous
series and thus recode yaml output on client side.

This will be done in next patches, for now simply limit
the usage of lua output to local sessions only.

Part-of #3834
---
 src/box/lua/console.lua | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index db57a6d80..b736eaa57 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -129,6 +129,28 @@ local function output_save(fmt, opts)
     }
 end
 
+local function output_verify_remote(greeting)
+    --
+    -- FIXME
+    -- For now we disable operating in Lua output
+    -- mode for remote connections, simply because
+    -- we need to extend output processing and pass
+    -- it into remote peer. Also for old servers which
+    -- won't support Lua more we will need to provide
+    -- compatibility layer and decode yaml manually.
+    --
+    -- Thus we require both output settings (per session
+    -- and by default to be set to yaml).
+    local d = box.session.storage.console_output
+    if (d ~= nil and d["fmt"] == "lua") or
+        (d == nil and ouput_default_handler["fmt"] == "lua") then
+        local msg1 = "Lua mode is not support right now. "
+        local msg2 = "Please switch back to yaml output."
+        return false, msg1 .. msg2
+    end
+    return true, nil
+end
+
 local function format(status, ...)
     local d = box.session.storage.console_output
     if d == nil then
@@ -599,6 +621,14 @@ local function connect(uri, opts)
         log.verbose(greeting)
         box.error(box.error.NO_CONNECTION)
     end
+
+    local ok, res = output_verify_remote(greeting)
+    if not ok then
+        connection:close()
+        pcall(self.on_client_disconnect, self)
+        error(res)
+    end
+
     local remote
     if greeting.protocol == 'Lua console' then
         remote = wrap_text_socket(connection, u,
-- 
2.20.1

[tarantool-patches] Re: [PATCH 5/5] box/lua/console: Limit lua output for local sessions only

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/07/24 01:35]:
> To make full support of lua mode on remote connection we
> need pass this option to remote server. Also we need to
> provide a compatibility layer for servers from previous
> series and thus recode yaml output on client side.
> 
> This will be done in next patches, for now simply limit
> the usage of lua output to local sessions only.
> 
> Part-of #3834
> ---
>  src/box/lua/console.lua | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)

I don't think this patch is necessary, or we need to do anything
about remote consoles. What we have is good enough - remote format
is used. We can simply document it.
Whoever doesn't like the remote format, can change it right after
connecting to the remote.

Not pushing.


-- 
Konstantin Osipov, Moscow, Russia

[tarantool-patches] Re: [PATCH 5/5] box/lua/console: Limit lua output for local sessions only

[Cyrill Gorcunov]

On Wed, Jul 24, 2019 at 08:15:37PM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@gmail.com> [19/07/24 01:35]:
> > To make full support of lua mode on remote connection we
> > need pass this option to remote server. Also we need to
> > provide a compatibility layer for servers from previous
> > series and thus recode yaml output on client side.
> > 
> > This will be done in next patches, for now simply limit
> > the usage of lua output to local sessions only.
> > 
> > Part-of #3834
> > ---
> >  src/box/lua/console.lua | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> 
> I don't think this patch is necessary, or we need to do anything
> about remote consoles. What we have is good enough - remote format
> is used. We can simply document it.
> Whoever doesn't like the remote format, can change it right after
> connecting to the remote.
> 
> Not pushing.

I see. Need to implement deep testing of all this.

[tarantool-patches] Re: [PATCH v4 0/5] box/lua/console: Add initial support for lua output format

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/07/24 01:35]:
> Hi, here is an updated series for lua console support. It is in devel state
> still since we need to enhance output support for remote connections. By
> now I limit this mode for local sessions only.
> 
> Main purpose of this series is to not break anything existing thus requires
> a deep testing.

I checked in some patches from this branch with minor remarks.

Please add tests.


> TODO:
>  - extend exchange protocol with remote nodes to pass current output
>    mode and get ack/nack to process
>  - add compat layer for old remote server which do not have lua output
>    support at all and will provide us yaml output regarding of anything
>  - rework "help" and turorial output since in lua mode they are simply
>    unreadable
>  - extend test engine so that new tests would use lua output by default
>    (should we?)

I agree with the todo. Yes, we should switch to this format
universally.


-- 
Konstantin Osipov, Moscow, Russia