Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [RFC v2 0/2] box/lua/console: Add support for lua output format
@ 2019-07-05 21:09 Cyrill Gorcunov
  2019-07-05 21:09 ` [tarantool-patches] [RFC 1/2] third_party/serpent: Add serpent repo Cyrill Gorcunov
  2019-07-05 21:09 ` [tarantool-patches] [RFC 2/2] box/lua/console: Add support for lua output format Cyrill Gorcunov
  0 siblings, 2 replies; 11+ messages in thread
From: Cyrill Gorcunov @ 2019-07-05 21:09 UTC (permalink / raw)
  To: tml; +Cc: Konstantin Osipov, Alexander Turenko, Cyrill Gorcunov

Please review once time permit. It is still RFC to gather opinions.

Cyrill Gorcunov (2):
  third_party/serpent: Add serpent repo
  box/lua/console: Add support for lua output format

 .gitmodules             |  3 ++
 src/box/CMakeLists.txt  |  1 +
 src/box/lua/console.c   | 26 ++++++++++++
 src/box/lua/console.lua | 90 ++++++++++++++++++++++++++++++++++++++++-
 src/lua/help_en_US.lua  |  1 +
 third_party/serpent     |  1 +
 6 files changed, 121 insertions(+), 1 deletion(-)
 create mode 160000 third_party/serpent

-- 
2.20.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] [RFC 1/2] third_party/serpent: Add serpent repo
  2019-07-05 21:09 [tarantool-patches] [RFC v2 0/2] box/lua/console: Add support for lua output format Cyrill Gorcunov
@ 2019-07-05 21:09 ` Cyrill Gorcunov
  2019-07-05 21:09 ` [tarantool-patches] [RFC 2/2] box/lua/console: Add support for lua output format Cyrill Gorcunov
  1 sibling, 0 replies; 11+ messages in thread
From: Cyrill Gorcunov @ 2019-07-05 21:09 UTC (permalink / raw)
  To: tml; +Cc: Konstantin Osipov, Alexander Turenko, 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] [RFC 2/2] box/lua/console: Add support for lua output format
  2019-07-05 21:09 [tarantool-patches] [RFC v2 0/2] box/lua/console: Add support for lua output format Cyrill Gorcunov
  2019-07-05 21:09 ` [tarantool-patches] [RFC 1/2] third_party/serpent: Add serpent repo Cyrill Gorcunov
@ 2019-07-05 21:09 ` Cyrill Gorcunov
  2019-07-05 22:59   ` [tarantool-patches] " Konstantin Osipov
  2019-07-09  3:19   ` Alexander Turenko
  1 sibling, 2 replies; 11+ messages in thread
From: Cyrill Gorcunov @ 2019-07-05 21:09 UTC (permalink / raw)
  To: tml; +Cc: Konstantin Osipov, Alexander Turenko, 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.
---
 src/box/CMakeLists.txt  |  1 +
 src/box/lua/console.c   | 26 ++++++++++++
 src/box/lua/console.lua | 90 ++++++++++++++++++++++++++++++++++++++++-
 src/lua/help_en_US.lua  |  1 +
 4 files changed, 117 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
-- 
2.20.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] Re: [RFC 2/2] box/lua/console: Add support for lua output format
  2019-07-05 21:09 ` [tarantool-patches] [RFC 2/2] box/lua/console: Add support for lua output format Cyrill Gorcunov
@ 2019-07-05 22:59   ` Konstantin Osipov
  2019-07-05 23:04     ` Konstantin Osipov
  2019-07-06  6:50     ` Cyrill Gorcunov
  2019-07-09  3:19   ` Alexander Turenko
  1 sibling, 2 replies; 11+ messages in thread
From: Konstantin Osipov @ 2019-07-05 22:59 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: tml, Alexander Turenko

* Cyrill Gorcunov <gorcunov@gmail.com> [19/07/06 00:13]:

LGTM. @totktonada, please provide a more thorough review.

One question is what happens to the output on client reconnect.
Perhaps we should cache it on the client as well (this is the same
issue with console language).

> 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.
> ---
>  src/box/CMakeLists.txt  |  1 +
>  src/box/lua/console.c   | 26 ++++++++++++
>  src/box/lua/console.lua | 90 ++++++++++++++++++++++++++++++++++++++++-
>  src/lua/help_en_US.lua  |  1 +
>  4 files changed, 117 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
> -- 
> 2.20.1
> 

-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] Re: [RFC 2/2] box/lua/console: Add support for lua output format
  2019-07-05 22:59   ` [tarantool-patches] " Konstantin Osipov
@ 2019-07-05 23:04     ` Konstantin Osipov
  2019-07-06  6:50     ` Cyrill Gorcunov
  1 sibling, 0 replies; 11+ messages in thread
From: Konstantin Osipov @ 2019-07-05 23:04 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: tml, Alexander Turenko

* Konstantin Osipov <kostja@tarantool.org> [19/07/06 01:59]:
> * Cyrill Gorcunov <gorcunov@gmail.com> [19/07/06 00:13]:
> 
> LGTM. @totktonada, please provide a more thorough review.
> 
> One question is what happens to the output on client reconnect.
> Perhaps we should cache it on the client as well (this is the same
> issue with console language).

This matter is obviously is not a blocker, can be worked on in a
follow up patch.

Last thing, Lua output should be the default in 2.x

-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] Re: [RFC 2/2] box/lua/console: Add support for lua output format
  2019-07-05 22:59   ` [tarantool-patches] " Konstantin Osipov
  2019-07-05 23:04     ` Konstantin Osipov
@ 2019-07-06  6:50     ` Cyrill Gorcunov
  2019-07-06 16:02       ` Konstantin Osipov
  1 sibling, 1 reply; 11+ messages in thread
From: Cyrill Gorcunov @ 2019-07-06  6:50 UTC (permalink / raw)
  To: Konstantin Osipov; +Cc: tml, Alexander Turenko

On Sat, Jul 06, 2019 at 01:59:08AM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@gmail.com> [19/07/06 00:13]:
> 
> LGTM. @totktonada, please provide a more thorough review.
> 
> One question is what happens to the output on client reconnect.

Since it gonna be new session the settings will be reset. I thought
also that we might need to provide some box.cgf option for default
output mode?

> Perhaps we should cache it on the client as well (this is the same
> issue with console language).

Well, seems so, still it is a bit unclear for me where to keep this
cached value.

As to "default lua" for 2.x series we need to think how to tune
up our test engine to setup yaml mode for all tests, since currently
test results are in yaml and we definitely don't wanna update every
test exsiting.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] Re: [RFC 2/2] box/lua/console: Add support for lua output format
  2019-07-06  6:50     ` Cyrill Gorcunov
@ 2019-07-06 16:02       ` Konstantin Osipov
  2019-07-06 20:35         ` Cyrill Gorcunov
  2019-07-09  3:46         ` Alexander Turenko
  0 siblings, 2 replies; 11+ messages in thread
From: Konstantin Osipov @ 2019-07-06 16:02 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: tml, Alexander Turenko

* Cyrill Gorcunov <gorcunov@gmail.com> [19/07/06 17:40]:
> On Sat, Jul 06, 2019 at 01:59:08AM +0300, Konstantin Osipov wrote:
> > * Cyrill Gorcunov <gorcunov@gmail.com> [19/07/06 00:13]:
> > 
> > LGTM. @totktonada, please provide a more thorough review.
> > 
> > One question is what happens to the output on client reconnect.
> 
> Since it gonna be new session the settings will be reset. I thought
> also that we might need to provide some box.cgf option for default
> output mode?

Yes, but it could be box.session setting, not box.cfg.


> > Perhaps we should cache it on the client as well (this is the same
> > issue with console language).
> 
> Well, seems so, still it is a bit unclear for me where to keep this
> cached value.

I think having a default is good enough.

> As to "default lua" for 2.x series we need to think how to tune
> up our test engine to setup yaml mode for all tests, since currently
> test results are in yaml and we definitely don't wanna update every
> test exsiting.

Well, at least every new test should use the new output. There
aren't that many results, and it's just results - not tests - so
they won't be hard to merge (re-run the test  and compare the
diff). Besides, 1.10 is more or less frozen, it accepts only minor
bugfixes. Now it's a perfect time.

-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] Re: [RFC 2/2] box/lua/console: Add support for lua output format
  2019-07-06 16:02       ` Konstantin Osipov
@ 2019-07-06 20:35         ` Cyrill Gorcunov
  2019-07-09  3:46         ` Alexander Turenko
  1 sibling, 0 replies; 11+ messages in thread
From: Cyrill Gorcunov @ 2019-07-06 20:35 UTC (permalink / raw)
  To: Konstantin Osipov; +Cc: tml, Alexander Turenko

On Sat, Jul 06, 2019 at 07:02:39PM +0300, Konstantin Osipov wrote:
> > 
> > Since it gonna be new session the settings will be reset. I thought
> > also that we might need to provide some box.cgf option for default
> > output mode?
> 
> Yes, but it could be box.session setting, not box.cfg.

OK, will take a look, thanks!

> > > Perhaps we should cache it on the client as well (this is the same
> > > issue with console language).
> > 
> > Well, seems so, still it is a bit unclear for me where to keep this
> > cached value.
> 
> I think having a default is good enough.

ok

> > As to "default lua" for 2.x series we need to think how to tune
> > up our test engine to setup yaml mode for all tests, since currently
> > test results are in yaml and we definitely don't wanna update every
> > test exsiting.
> 
> Well, at least every new test should use the new output. There
> aren't that many results, and it's just results - not tests - so
> they won't be hard to merge (re-run the test  and compare the
> diff). Besides, 1.10 is more or less frozen, it accepts only minor
> bugfixes. Now it's a perfect time.

got it, thanks!

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] Re: [RFC 2/2] box/lua/console: Add support for lua output format
  2019-07-05 21:09 ` [tarantool-patches] [RFC 2/2] box/lua/console: Add support for lua output format Cyrill Gorcunov
  2019-07-05 22:59   ` [tarantool-patches] " Konstantin Osipov
@ 2019-07-09  3:19   ` Alexander Turenko
  1 sibling, 0 replies; 11+ messages in thread
From: Alexander Turenko @ 2019-07-09  3:19 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: tml, Konstantin Osipov

> 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

AFAIR, this output is checked somewhere in our test suite (a test or a
test result file need to reflect this change).

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] Re: [RFC 2/2] box/lua/console: Add support for lua output format
  2019-07-06 16:02       ` Konstantin Osipov
  2019-07-06 20:35         ` Cyrill Gorcunov
@ 2019-07-09  3:46         ` Alexander Turenko
  2019-07-09 18:06           ` Konstantin Osipov
  1 sibling, 1 reply; 11+ messages in thread
From: Alexander Turenko @ 2019-07-09  3:46 UTC (permalink / raw)
  To: Konstantin Osipov; +Cc: Cyrill Gorcunov, tml

On Sat, Jul 06, 2019 at 07:02:39PM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@gmail.com> [19/07/06 17:40]:
> > On Sat, Jul 06, 2019 at 01:59:08AM +0300, Konstantin Osipov wrote:
> > > * Cyrill Gorcunov <gorcunov@gmail.com> [19/07/06 00:13]:
> > > 
> > > LGTM. @totktonada, please provide a more thorough review.

Okay.

> > > 
> > > One question is what happens to the output on client reconnect.
> > 
> > Since it gonna be new session the settings will be reset. I thought
> > also that we might need to provide some box.cgf option for default
> > output mode?
> 
> Yes, but it could be box.session setting, not box.cfg.

Related questions I'm tentative about:

* Is there a way to keep an old session after reconnection?
* Is there way to share a session for a connection pool?

> 
> 
> > > Perhaps we should cache it on the client as well (this is the same
> > > issue with console language).
> > 
> > Well, seems so, still it is a bit unclear for me where to keep this
> > cached value.
> 
> I think having a default is good enough.
> 
> > As to "default lua" for 2.x series we need to think how to tune
> > up our test engine to setup yaml mode for all tests, since currently
> > test results are in yaml and we definitely don't wanna update every
> > test exsiting.
> 
> Well, at least every new test should use the new output. There
> aren't that many results, and it's just results - not tests - so
> they won't be hard to merge (re-run the test  and compare the
> diff). Besides, 1.10 is more or less frozen, it accepts only minor
> bugfixes. Now it's a perfect time.

I guess the lua output format support will not be part of 1.10 branch?
If we'll change all test result files, then cherry-picking anything
backward to 1.10 will require extra work. So I don't think we should
convert old result files. 1.10 is alive, so I doubt it actually can be
frozen.

Re using lua output for new test result files. We'll need to write an
output format into a result file header. This requires to bump a result
file format version and implement the new logic in test-run only for
them (I don't see other backward compatible ways for now).

I'm also afraid that the inconsistency in result file formats (one of
which is not supported by 1.10) can give us problems if we'll decide to
backport something to 1.10 after it will land to master with a test in
the new format.

So lua output by default for new tests seems to be doable feature, but
will increase complexity of our test harness and will possibly
problematic in some (rare?) cases.

Are there any real problems with yaml result files?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] Re: [RFC 2/2] box/lua/console: Add support for lua output format
  2019-07-09  3:46         ` Alexander Turenko
@ 2019-07-09 18:06           ` Konstantin Osipov
  0 siblings, 0 replies; 11+ messages in thread
From: Konstantin Osipov @ 2019-07-09 18:06 UTC (permalink / raw)
  To: Alexander Turenko; +Cc: Cyrill Gorcunov, tml

* Alexander Turenko <alexander.turenko@tarantool.org> [19/07/09 09:46]:
> Related questions I'm tentative about:
> 
> * Is there a way to keep an old session after reconnection?

In theory we can delay destroying the session, but we wouldn't
know for how long.

> * Is there way to share a session for a connection pool?

That would be absurd. the whole point of having a session is to
have a private state.

We plan to multiplex sessions in future with streams btw.

> > Well, at least every new test should use the new output. There
> > aren't that many results, and it's just results - not tests - so
> > they won't be hard to merge (re-run the test  and compare the
> > diff). Besides, 1.10 is more or less frozen, it accepts only minor
> > bugfixes. Now it's a perfect time.
> 
> I guess the lua output format support will not be part of 1.10 branch?

No, we could only add \set output keyword support there, but not
the format itself. But there is very low risk of adding a new
output there too - so up to you.

> If we'll change all test result files, then cherry-picking anything
> backward to 1.10 will require extra work. So I don't think we should
> convert old result files. 1.10 is alive, so I doubt it actually can be
> frozen.

then let's add this to both branches.

> Re using lua output for new test result files. We'll need to write an
> output format into a result file header. This requires to bump a result
> file format version and implement the new logic in test-run only for
> them (I don't see other backward compatible ways for now).
> 
> I'm also afraid that the inconsistency in result file formats (one of
> which is not supported by 1.10) can give us problems if we'll decide to
> backport something to 1.10 after it will land to master with a test in
> the new format.
> 
> So lua output by default for new tests seems to be doable feature, but
> will increase complexity of our test harness and will possibly
> problematic in some (rare?) cases.
> 
> Are there any real problems with yaml result files?

yes, the complexity of having too many result file format. Long
term we don't need this complexity, it's a technical debt. Same
applies to the current sql tests, most of them can be in plain sql.

-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-07-09 18:06 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-05 21:09 [tarantool-patches] [RFC v2 0/2] box/lua/console: Add support for lua output format Cyrill Gorcunov
2019-07-05 21:09 ` [tarantool-patches] [RFC 1/2] third_party/serpent: Add serpent repo Cyrill Gorcunov
2019-07-05 21:09 ` [tarantool-patches] [RFC 2/2] box/lua/console: Add support for lua output format Cyrill Gorcunov
2019-07-05 22:59   ` [tarantool-patches] " Konstantin Osipov
2019-07-05 23:04     ` Konstantin Osipov
2019-07-06  6:50     ` Cyrill Gorcunov
2019-07-06 16:02       ` Konstantin Osipov
2019-07-06 20:35         ` Cyrill Gorcunov
2019-07-09  3:46         ` Alexander Turenko
2019-07-09 18:06           ` Konstantin Osipov
2019-07-09  3:19   ` Alexander Turenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox