* [tarantool-patches] [PATCH v1 1/1] Tarantoolctl "enter" with set language @ 2018-07-25 7:51 imeevma 2018-07-25 21:56 ` [tarantool-patches] " Vladislav Shpilevoy 0 siblings, 1 reply; 4+ messages in thread From: imeevma @ 2018-07-25 7:51 UTC (permalink / raw) To: tarantool-patches This patch allow to use option "--language" with "tarantoolctl enter" command. Also default value for language were added in default configuration file. Language is either SQL or Lua. Closes #2385. --- Branch: https://github.com/tarantool/tarantool/tree/imeevma/gh-2385-select-input-language-tarantoolctl Issue: https://github.com/tarantool/tarantool/issues/2385 extra/dist/CMakeLists.txt | 1 + extra/dist/default/tarantool.in | 1 + extra/dist/tarantoolctl.in | 22 +++++++++++++++++++++- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/extra/dist/CMakeLists.txt b/extra/dist/CMakeLists.txt index 862689e..3045b5a 100644 --- a/extra/dist/CMakeLists.txt +++ b/extra/dist/CMakeLists.txt @@ -34,6 +34,7 @@ message (STATUS "tarantoolctl logdir: ${TARANTOOL_LOGDIR}") set(TARANTOOL_RUNDIR "${CMAKE_INSTALL_FULL_LOCALSTATEDIR}/run/tarantool") message (STATUS "tarantoolctl rundir: ${TARANTOOL_RUNDIR}") set(TARANTOOL_USER "tarantool") +set(TARANTOOL_LANGUAGE "lua") set(SYSCONFIG_AVAILABLEDIR "tarantool/instances.available") set(SYSCONFIG_ENABLEDDIR "tarantool/instances.enabled") set(TARANTOOL_AVAILABLEDIR "${CMAKE_INSTALL_FULL_SYSCONFDIR}/${SYSCONFIG_AVAILABLEDIR}") diff --git a/extra/dist/default/tarantool.in b/extra/dist/default/tarantool.in index 9a8dad3..01e7144 100644 --- a/extra/dist/default/tarantool.in +++ b/extra/dist/default/tarantool.in @@ -20,6 +20,7 @@ default_cfg = { vinyl_dir = "@TARANTOOL_DATADIR@", -- @TARANTOOL_DATADIR@/${INSTANCE} log = "@TARANTOOL_LOGDIR@", -- @TARANTOOL_LOGDIR@/${INSTANCE}.log username = "@TARANTOOL_USER@", + language = "@TARANTOOL_LANGUAGE@", } -- instances.available - all available instances diff --git a/extra/dist/tarantoolctl.in b/extra/dist/tarantoolctl.in index 2dd5d74..55711b8 100755 --- a/extra/dist/tarantoolctl.in +++ b/extra/dist/tarantoolctl.in @@ -155,6 +155,11 @@ local function load_default_file(default_file) d.vinyl_dir = fio.pathjoin(d.vinyl_dir, instance_name) d.log = fio.pathjoin(d.log, instance_name .. '.log') + d.language = (d.language or "lua"):lower() + if d.language ~= "lua" and d.language ~= "sql" then + d.language = "lua" + end + default_cfg = d if not usermode then @@ -454,6 +459,7 @@ local function wrapper_cfg(cfg) elseif cfg.background == nil then cfg.background = true end + cfg.language = nil mk_default_dirs(cfg) local success, data = pcall(orig_cfg, cfg) @@ -629,6 +635,17 @@ local function logrotate() end local function enter() + local options = keyword_arguments + local language = options.language + if language ~= nil then + language = language:lower() + if language ~= "lua" and language ~= "sql" then + log.warn("Language \"%s\" is not recognized. Default language" .. + " \"%s\" is set.", language, default_cfg.language) + language = nil + end + end + language = language or default_cfg.language local console_sock_path = uri.parse(console_sock).service if fio.stat(console_sock_path) == nil then log.error("Can't connect to %s (%s)", console_sock_path, errno.strerror()) @@ -644,7 +661,9 @@ local function enter() console_sock, TIMEOUT_INFINITY ) - console.on_start(function(self) self:eval(cmd) end) + local cmd_language = string.format("\\set language %s", language) + + console.on_start(function(self) self:eval(cmd) self:eval(cmd_language) end) console.on_client_disconnect(function(self) self.running = false end) console.start() return 0 @@ -1284,6 +1303,7 @@ do { 'chdir', 'string' }, { 'only-server', 'string' }, { 'server', 'string' }, + { 'language', 'string' }, }) local cmd_name -- 2.7.4 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] Tarantoolctl "enter" with set language 2018-07-25 7:51 [tarantool-patches] [PATCH v1 1/1] Tarantoolctl "enter" with set language imeevma @ 2018-07-25 21:56 ` Vladislav Shpilevoy 2018-07-26 11:41 ` Imeev Mergen 0 siblings, 1 reply; 4+ messages in thread From: Vladislav Shpilevoy @ 2018-07-25 21:56 UTC (permalink / raw) To: tarantool-patches, imeevma On 25/07/2018 10:51, imeevma@tarantool.org wrote: > This patch allow to use option "--language" with > "tarantoolctl enter" command. Also default value > for language were added in default configuration > file. Language is either SQL or Lua. > > Closes #2385. > --- > Branch: https://github.com/tarantool/tarantool/tree/imeevma/gh-2385-select-input-language-tarantoolctl > Issue: https://github.com/tarantool/tarantool/issues/2385 > > diff --git a/extra/dist/tarantoolctl.in b/extra/dist/tarantoolctl.in > index 2dd5d74..55711b8 100755 > --- a/extra/dist/tarantoolctl.in > +++ b/extra/dist/tarantoolctl.in > @@ -155,6 +155,11 @@ local function load_default_file(default_file) > d.vinyl_dir = fio.pathjoin(d.vinyl_dir, instance_name) > d.log = fio.pathjoin(d.log, instance_name .. '.log') > > + d.language = (d.language or "lua"):lower() > + if d.language ~= "lua" and d.language ~= "sql" then > + d.language = "lua" 1. Please, do not ignore unknown language. Log this error like others in the same function already do. 2. Why do you need language in default_cfg? It is box.cfg as I understand and box.cfg has no language option. It is not? If you need to parse a language from the file, then you can store it in another table or even in a separate variable like others on lines 30-49. > + end > + > default_cfg = d > > if not usermode the> @@ -629,6 +635,17 @@ local function logrotate() > end > > local function enter() > + local options = keyword_arguments > + local language = options.language > + if language ~= nil then > + language = language:lower() > + if language ~= "lua" and language ~= "sql" then > + log.warn("Language \"%s\" is not recognized. Default language" .. > + " \"%s\" is set.", language, default_cfg.language) > + language = nil 3. For enter() function it is ok to fail on unknown language. You should not ignore the error because a caller could use something like this: cat my_script.sql | tarantoolctl enter --language sqlll And if you ignore invalid language, this command will feed the entire script on SQL into Lua console. It will produce huge number of syntax errors if no worse. > + end > + end > + language = language or default_cfg.language > local console_sock_path = uri.parse(console_sock).service > if fio.stat(console_sock_path) == nil then > log.error("Can't connect to %s (%s)", console_sock_path, errno.strerror()) > @@ -644,7 +661,9 @@ local function enter() > console_sock, TIMEOUT_INFINITY > ) > > - console.on_start(function(self) self:eval(cmd) end) > + local cmd_language = string.format("\\set language %s", language) 4. Why not to merge cmd_language into cmd? It is created a line above. > + > + console.on_start(function(self) self:eval(cmd) self:eval(cmd_language) end) > console.on_client_disconnect(function(self) self.running = false end) > console.start() > return 0 > @@ -1284,6 +1303,7 @@ do > { 'chdir', 'string' }, > { 'only-server', 'string' }, > { 'server', 'string' }, > + { 'language', 'string' }, > }) > > local cmd_name > 5. Lets add the same option to 'eval', 'connect' to make the commands symmetric. 6. Please, update the comments. For example, on line 1028 I still see the comment about 'enter': "Enter an instance's interactive Lua console", but actually it is not complete now - you can choose SQL. ^ permalink raw reply [flat|nested] 4+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] Tarantoolctl "enter" with set language 2018-07-25 21:56 ` [tarantool-patches] " Vladislav Shpilevoy @ 2018-07-26 11:41 ` Imeev Mergen 2018-07-27 13:08 ` Vladislav Shpilevoy 0 siblings, 1 reply; 4+ messages in thread From: Imeev Mergen @ 2018-07-26 11:41 UTC (permalink / raw) To: Vladislav Shpilevoy, tarantool-patches [-- Attachment #1: Type: text/plain, Size: 8667 bytes --] On 07/26/2018 12:56 AM, Vladislav Shpilevoy wrote: > > > On 25/07/2018 10:51, imeevma@tarantool.org wrote: >> This patch allow to use option "--language" with >> "tarantoolctl enter" command. Also default value >> for language were added in default configuration >> file. Language is either SQL or Lua. >> >> Closes #2385. >> --- >> Branch: >> https://github.com/tarantool/tarantool/tree/imeevma/gh-2385-select-input-language-tarantoolctl >> >> Issue: https://github.com/tarantool/tarantool/issues/2385 >> >> diff --git a/extra/dist/tarantoolctl.in b/extra/dist/tarantoolctl.in >> index 2dd5d74..55711b8 100755 >> --- a/extra/dist/tarantoolctl.in >> +++ b/extra/dist/tarantoolctl.in >> @@ -155,6 +155,11 @@ local function load_default_file(default_file) >> d.vinyl_dir = fio.pathjoin(d.vinyl_dir, instance_name) >> d.log = fio.pathjoin(d.log, instance_name .. '.log') >> + d.language = (d.language or "lua"):lower() >> + if d.language ~= "lua" and d.language ~= "sql" then >> + d.language = "lua" > > 1. Please, do not ignore unknown language. Log this error like others > in the same function already do. Done. > > 2. Why do you need language in default_cfg? It is box.cfg as I > understand and box.cfg has no language option. It is not? If you > need to parse a language from the file, then you can store it in > another table or even in a separate variable like others on lines > 30-49. Done. > >> + end >> + >> default_cfg = d >> if not usermode the> @@ -629,6 +635,17 @@ local function >> logrotate() >> end >> local function enter() >> + local options = keyword_arguments >> + local language = options.language >> + if language ~= nil then >> + language = language:lower() >> + if language ~= "lua" and language ~= "sql" then >> + log.warn("Language \"%s\" is not recognized. Default >> language" .. >> + " \"%s\" is set.", language, default_cfg.language) >> + language = nil > > 3. For enter() function it is ok to fail on unknown language. > You should not ignore the error because a caller could use > something like this: > > cat my_script.sql | tarantoolctl enter --language sqlll > > And if you ignore invalid language, this command will feed the > entire script on SQL into Lua console. It will produce huge > number of syntax errors if no worse. Done. > >> + end >> + end >> + language = language or default_cfg.language >> local console_sock_path = uri.parse(console_sock).service >> if fio.stat(console_sock_path) == nil then >> log.error("Can't connect to %s (%s)", console_sock_path, >> errno.strerror()) >> @@ -644,7 +661,9 @@ local function enter() >> console_sock, TIMEOUT_INFINITY >> ) >> - console.on_start(function(self) self:eval(cmd) end) >> + local cmd_language = string.format("\\set language %s", language) > > 4. Why not to merge cmd_language into cmd? It is created a line above. Discussed, partly changed. > >> + >> + console.on_start(function(self) self:eval(cmd) >> self:eval(cmd_language) end) >> console.on_client_disconnect(function(self) self.running = >> false end) >> console.start() >> return 0 >> @@ -1284,6 +1303,7 @@ do >> { 'chdir', 'string' }, >> { 'only-server', 'string' }, >> { 'server', 'string' }, >> + { 'language', 'string' }, >> }) >> local cmd_name >> > > 5. Lets add the same option to 'eval', 'connect' to make the commands > symmetric. Discussed, decided to reject the idea. > > 6. Please, update the comments. For example, on line 1028 I > still see the comment about 'enter': "Enter an instance's interactive > Lua console", > but actually it is not complete now - you can choose SQL. > Done. commit 7088606bba3810ffc6db313cf31d7aa101d7175d Author: Mergen Imeev <imeevma@gmail.com> Date: Fri Jul 13 19:25:32 2018 +0300 Tarantoolctl "enter" with set language This patch allow to use option "--language" with "tarantoolctl enter" command. Also default value for language were added in default configuration file. Language is either SQL or Lua. Closes #2385. diff --git a/extra/dist/CMakeLists.txt b/extra/dist/CMakeLists.txt index 862689e..3045b5a 100644 --- a/extra/dist/CMakeLists.txt +++ b/extra/dist/CMakeLists.txt @@ -34,6 +34,7 @@ message (STATUS "tarantoolctl logdir: ${TARANTOOL_LOGDIR}") set(TARANTOOL_RUNDIR "${CMAKE_INSTALL_FULL_LOCALSTATEDIR}/run/tarantool") message (STATUS "tarantoolctl rundir: ${TARANTOOL_RUNDIR}") set(TARANTOOL_USER "tarantool") +set(TARANTOOL_LANGUAGE "lua") set(SYSCONFIG_AVAILABLEDIR "tarantool/instances.available") set(SYSCONFIG_ENABLEDDIR "tarantool/instances.enabled") set(TARANTOOL_AVAILABLEDIR "${CMAKE_INSTALL_FULL_SYSCONFDIR}/${SYSCONFIG_AVAILABLEDIR}") diff --git a/extra/dist/default/tarantool.in b/extra/dist/default/tarantool.in index 9a8dad3..01e7144 100644 --- a/extra/dist/default/tarantool.in +++ b/extra/dist/default/tarantool.in @@ -20,6 +20,7 @@ default_cfg = { vinyl_dir = "@TARANTOOL_DATADIR@", -- @TARANTOOL_DATADIR@/${INSTANCE} log = "@TARANTOOL_LOGDIR@", -- @TARANTOOL_LOGDIR@/${INSTANCE}.log username = "@TARANTOOL_USER@", + language = "@TARANTOOL_LANGUAGE@", } -- instances.available - all available instances diff --git a/extra/dist/tarantoolctl.in b/extra/dist/tarantoolctl.in index 2dd5d74..ef9da2f 100755 --- a/extra/dist/tarantoolctl.in +++ b/extra/dist/tarantoolctl.in @@ -47,6 +47,7 @@ local default_cfg local positional_arguments local keyword_arguments local lua_arguments = arg +local language -- function for printing usage reference local usage @@ -155,6 +156,14 @@ local function load_default_file(default_file) d.vinyl_dir = fio.pathjoin(d.vinyl_dir, instance_name) d.log = fio.pathjoin(d.log, instance_name .. '.log') + language = (d.language or "lua"):lower() + d.language = nil + + if language ~= "lua" and language ~= "sql" then + log.error('Unknown language: %s', language) + os.exit(1) + end + default_cfg = d if not usermode then @@ -629,6 +638,12 @@ local function logrotate() end local function enter() + local options = keyword_arguments + language = (options.language or language):lower() + if language ~= "lua" and language ~= "sql" then + log.error('Unknown language: %s', options.language) + return 1 + end local console_sock_path = uri.parse(console_sock).service if fio.stat(console_sock_path) == nil then log.error("Can't connect to %s (%s)", console_sock_path, errno.strerror()) @@ -644,7 +659,9 @@ local function enter() console_sock, TIMEOUT_INFINITY ) - console.on_start(function(self) self:eval(cmd) end) + local cmd_connect = string.format("\\set language %s", language) + + console.on_start(function(self) self:eval(cmd) self:eval(cmd_connect) end) console.on_client_disconnect(function(self) self.running = false end) console.start() return 0 @@ -1008,11 +1025,15 @@ local commands = setmetatable({ } }, enter = { func = exit_wrapper(enter), process = process_local, help = { - header = "%s enter INSTANCE", + header = "%s enter INSTANCE [--language=language]", linkmode = "%s enter", description = [=[ - Enter an instance's interactive Lua console. + Enter an instance's interactive Lua or SQL console. + + Supported options: + * --language=language to set interactive console language. + May be either Lua or SQL. ]=], weight = 65, deprecated = false, @@ -1284,6 +1305,7 @@ do { 'chdir', 'string' }, { 'only-server', 'string' }, { 'server', 'string' }, + { 'language', 'string' }, }) local cmd_name [-- Attachment #2: Type: text/html, Size: 14072 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] Tarantoolctl "enter" with set language 2018-07-26 11:41 ` Imeev Mergen @ 2018-07-27 13:08 ` Vladislav Shpilevoy 0 siblings, 0 replies; 4+ messages in thread From: Vladislav Shpilevoy @ 2018-07-27 13:08 UTC (permalink / raw) To: Imeev Mergen, tarantool-patches Hi! Thanks for the fixes! > diff --git a/extra/dist/tarantoolctl.in b/extra/dist/tarantoolctl.in > index 2dd5d74..ef9da2f 100755 > --- a/extra/dist/tarantoolctl.in > +++ b/extra/dist/tarantoolctl.in > log.error("Can't connect to %s (%s)", console_sock_path, errno.strerror()) > @@ -644,7 +659,9 @@ local function enter() > console_sock, TIMEOUT_INFINITY > ) > > - console.on_start(function(self) self:eval(cmd) end) > + local cmd_connect = string.format("\\set language %s", language) 1. It is not cmd_connect, it is cmd_language. Connect cmd is above. > + > + console.on_start(function(self) self:eval(cmd) self:eval(cmd_connect) end) > console.on_client_disconnect(function(self) self.running = false end) > console.start() > return 0 2. Please, write a documentation bot request for the new option. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-07-27 13:08 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-07-25 7:51 [tarantool-patches] [PATCH v1 1/1] Tarantoolctl "enter" with set language imeevma 2018-07-25 21:56 ` [tarantool-patches] " Vladislav Shpilevoy 2018-07-26 11:41 ` Imeev Mergen 2018-07-27 13:08 ` Vladislav Shpilevoy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox