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

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

[Cyrill Gorcunov]

Hi, here is a v3 of initial support for lua output format. This is still in
devel state so I think we will need more patches to cover all cases and provide
deep testing. Since it will take some time to proceed I think better to move
with small steps. Thus for this series the most important target is to not break
anything existing.

We have so far

 - default output mode remains set to yaml
 - test/box/admin updated to cover new \help command output
 - when connecting to server which is old enough and doesn't
   support binary protocol we issue an error so that lua output
   should not be used in the case

---
The following changes since commit 77051a11029560eee8ad0adf83c8ac92e64a7d12:

  box: do not check state in case of reconnect (2019-07-12 12:07:02 +0300)

are available in the Git repository at:

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

for you to fetch changes up to 712e1ac1b31798ac4348bbf6d84833b71854e957:

  box/lua/console: Verify remote protocol for compatibility (2019-07-12 20:53:47 +0300)

----------------------------------------------------------------
Cyrill Gorcunov (3):
      third_party/serpent: Add serpent repo
      box/lua/console: Add support for lua output format
      box/lua/console: Verify remote protocol for compatibility

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

-- 
2.20.1

[tarantool-patches] [PATCH 1/3] 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] [PATCH 2/3] 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 | 90 ++++++++++++++++++++++++++++++++++++++++-
 src/lua/help_en_US.lua  |  1 +
 test/box/admin.result   |  1 +
 5 files changed, 118 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..7c3b608cb 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,71 @@ 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
+    end
+    if fmt == "lua" then
+        if opts ~= "line" and opts ~= "block" then
+            local msg = 'Wrong option "%s", expecting: line or block.'
+            error(msg:format(opts))
+        end
+    end
+end
+
+local function output_save(fmt, opts)
+    --
+    -- Output format descriptors are saved per
+    -- session thus each console may specify own
+    -- specifics.
+    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 +139,31 @@ local function set_language(storage, value)
     return true
 end
 
+local function set_output(storage, 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
+            output_verify_opts(fmt, opts)
+            output_save(fmt, opts)
+            return true
+        end
+    end
+    local msg = 'Invalid format "%s", supported languages: lua and yaml.'
+    return error(msg:format(value))
+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] Re: [PATCH 2/3] box/lua/console: Add support for lua output format

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/07/12 22:08]:
> 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.

Can you please come up with a patch for test-run that changes the
test engine to set the output explicitly in suite.ini, so that
it's not broken by changing the default?


-- 
Konstantin Osipov, Moscow, Russia

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

[Cyrill Gorcunov]

On Fri, Jul 12, 2019 at 11:45:27PM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@gmail.com> [19/07/12 22:08]:
> > 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.
> 
> Can you please come up with a patch for test-run that changes the
> test engine to set the output explicitly in suite.ini, so that
> it's not broken by changing the default?

I think about it and this gonna be new patches on top of the series.
Look, the main purpose of the patchset is to be completely transparent
to current codebase and the patches must not break anything already
running (including tests). That's why modifying test engine is my
next step, also I need to provide tests for this new console output.
But again, my initial main purpose -- to not break anything now.

[tarantool-patches] [PATCH 3/3] box/lua/console: Verify remote protocol for compatibility

[Cyrill Gorcunov]

New versions of tarantool use binary protocol to negotiate
with remote servers where all data can be properly serialized
by the serpent engine.

In turn old instances continue shipping yaml encoded data via
text protocol which we simply can't map into lua form. Thus
when we connect to a peer make sure that the data it will
provide us is supported.

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

diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index 7c3b608cb..a9ff6f9a7 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -87,6 +87,22 @@ local function output_save(fmt, opts)
     }
 end
 
+local function output_verify_remote(greeting)
+    --
+    -- In case if remote peer is operating over
+    -- old yaml protocol we are unable to decode
+    -- it to non-yaml serializers.
+    if greeting.protocol == 'Lua console' then
+        local d = box.session.storage.console_output
+        if d ~= nil and d["fmt"] == "lua" then
+            local msg1 = "Peer uses deprecated '%s' protocol. "
+            local msg2 = "Switch output to 'yaml' format to proceed."
+            return false, msg1:format(greeting.protocol) .. msg2
+        end
+    end
+    return true, nil
+end
+
 local function format(status, ...)
     local d = box.session.storage.console_output
     --
@@ -572,6 +588,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 3/3] box/lua/console: Verify remote protocol for compatibility

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/07/12 22:08]:
> +local function output_verify_remote(greeting)
> +    --
> +    -- In case if remote peer is operating over
> +    -- old yaml protocol we are unable to decode
> +    -- it to non-yaml serializers.
> +    if greeting.protocol == 'Lua console' then
> +        local d = box.session.storage.console_output
> +        if d ~= nil and d["fmt"] == "lua" then
> +            local msg1 = "Peer uses deprecated '%s' protocol. "
> +            local msg2 = "Switch output to 'yaml' format to proceed."
> +            return false, msg1:format(greeting.protocol) .. msg2
> +        end
> +    end
> +    return true, nil
> +end

what is the problem with doing yaml.load and serpent.encode for
such peers? 

What if it's a new peer but using yaml output?

-- 
Konstantin Osipov, Moscow, Russia

[tarantool-patches] Re: [PATCH 3/3] box/lua/console: Verify remote protocol for compatibility

[Konstantin Osipov]

* Konstantin Osipov <kostja@tarantool.org> [19/07/12 23:47]:
> > +local function output_verify_remote(greeting)
> > +    --
> > +    -- In case if remote peer is operating over
> > +    -- old yaml protocol we are unable to decode
> > +    -- it to non-yaml serializers.
> > +    if greeting.protocol == 'Lua console' then
> > +        local d = box.session.storage.console_output
> > +        if d ~= nil and d["fmt"] == "lua" then
> > +            local msg1 = "Peer uses deprecated '%s' protocol. "
> > +            local msg2 = "Switch output to 'yaml' format to proceed."
> > +            return false, msg1:format(greeting.protocol) .. msg2
> > +        end
> > +    end
> > +    return true, nil
> > +end
> 
> what is the problem with doing yaml.load and serpent.encode for
> such peers? 
> 
> What if it's a new peer but using yaml output?

Moreover, shouldn't the output setting affect only the local
server output, but not subsequent console.connect() calls? 

-- 
Konstantin Osipov, Moscow, Russia

[tarantool-patches] Re: [PATCH 3/3] box/lua/console: Verify remote protocol for compatibility

[Cyrill Gorcunov]

On Fri, Jul 12, 2019 at 11:49:04PM +0300, Konstantin Osipov wrote:
> * Konstantin Osipov <kostja@tarantool.org> [19/07/12 23:47]:
> > > +local function output_verify_remote(greeting)
> > > +    --
> > > +    -- In case if remote peer is operating over
> > > +    -- old yaml protocol we are unable to decode
> > > +    -- it to non-yaml serializers.
> > > +    if greeting.protocol == 'Lua console' then
> > > +        local d = box.session.storage.console_output
> > > +        if d ~= nil and d["fmt"] == "lua" then
> > > +            local msg1 = "Peer uses deprecated '%s' protocol. "
> > > +            local msg2 = "Switch output to 'yaml' format to proceed."
> > > +            return false, msg1:format(greeting.protocol) .. msg2
> > > +        end
> > > +    end
> > > +    return true, nil
> > > +end
> > 
> > what is the problem with doing yaml.load and serpent.encode for
> > such peers?

'Lua console' proto is depecated, binary proto is preferred and
I think we may simply get rid it, otherwise a way more code is
needed here which I would try to escape.

> > 
> > What if it's a new peer but using yaml output?

Every new connection does this check so that if peer is
reporting old text protocol we refuse to connect until
local output set back to yaml.

> 
> Moreover, shouldn't the output setting affect only the local
> server output, but not subsequent console.connect() calls? 

Not sure I follow here. This code checks for reply from remote
peer, every output bound to a session, and every connection
is a new session (no?) thus we do test each time. Or you meant
something different?