* [Tarantool-patches] [PATCH] test_run: add path option to grep_log
@ 2020-02-18 18:01 Olga Arkhangelskaia
2020-03-04 13:51 ` Alexander Turenko
0 siblings, 1 reply; 5+ messages in thread
From: Olga Arkhangelskaia @ 2020-02-18 18:01 UTC (permalink / raw)
To: tarantool-patches
While using grep_log it is useful to have a possibility to go through crashed
instance log.
Usage in case of crashed instance:
test_run:grep_log(nil, str-to-find, nil, {path=path_to_log})
Closes #196
---
Issue:https://github.com/tarantool/test-run/issues/196
Branch:https://github.com/tarantool/test-run/pull/new/OKriw/gh-196-Extend-grep_log-for-crashed-instances
test_run.lua | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/test_run.lua b/test_run.lua
index 4d8132d..68dba53 100644
--- a/test_run.lua
+++ b/test_run.lua
@@ -326,7 +326,9 @@ end
local function grep_log(self, node, what, bytes, opts)
local opts = opts or {}
local noreset = opts.noreset or false
- local filename = self:eval(node, "box.cfg.log")[1]
+ -- node is used in case of alive instance, otherwise path to log can be
+ -- used for path
+ local filename = opts.path or self:eval(node, "box.cfg.log")[1]
local file = fio.open(filename, {'O_RDONLY', 'O_NONBLOCK'})
local function fail(msg)
--
2.20.1 (Apple Git-117)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Tarantool-patches] [PATCH] test_run: add path option to grep_log
2020-02-18 18:01 [Tarantool-patches] [PATCH] test_run: add path option to grep_log Olga Arkhangelskaia
@ 2020-03-04 13:51 ` Alexander Turenko
2020-03-10 11:29 ` Olga Arkhangelskaia
0 siblings, 1 reply; 5+ messages in thread
From: Alexander Turenko @ 2020-03-04 13:51 UTC (permalink / raw)
To: Olga Arkhangelskaia; +Cc: tarantool-patches
The patch looks okay, but I would ask you to do few small modifications
(see below).
WBR, Alexander Turenko.
On Tue, Feb 18, 2020 at 09:01:01PM +0300, Olga Arkhangelskaia wrote:
> While using grep_log it is useful to have a possibility to go through crashed
Please, keep a commit message body within 72 symbols.
See https://www.tarantool.io/en/doc/1.10/dev_guide/developer_guidelines/#how-to-write-a-commit-message
> instance log.
> Usage in case of crashed instance:
> test_run:grep_log(nil, str-to-find, nil, {path=path_to_log})
>
> Closes #196
> ---
> Issue:https://github.com/tarantool/test-run/issues/196
> Branch:https://github.com/tarantool/test-run/pull/new/OKriw/gh-196-Extend-grep_log-for-crashed-instances
>
> test_run.lua | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/test_run.lua b/test_run.lua
> index 4d8132d..68dba53 100644
> --- a/test_run.lua
> +++ b/test_run.lua
> @@ -326,7 +326,9 @@ end
> local function grep_log(self, node, what, bytes, opts)
> local opts = opts or {}
> local noreset = opts.noreset or false
> - local filename = self:eval(node, "box.cfg.log")[1]
> + -- node is used in case of alive instance, otherwise path to log can be
> + -- used for path
> + local filename = opts.path or self:eval(node, "box.cfg.log")[1]
Nit: It is rule of thumb to fit comments within 66 symbols.
We already use 'filename' name for this, I would use it for the option
too.
I suggest the following pattern for options:
| local function f(<...>, opts)
| local opts = opts or {}
| local foo = opts.foo (or <default_value> if it is not nil)
| local bar = opts.bar
| <...the rest code...>
| end
This way it is easy to find them all in code at one glance.
> local file = fio.open(filename, {'O_RDONLY', 'O_NONBLOCK'})
>
> local function fail(msg)
> --
> 2.20.1 (Apple Git-117)
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Tarantool-patches] [PATCH] test_run: add path option to grep_log
2020-03-04 13:51 ` Alexander Turenko
@ 2020-03-10 11:29 ` Olga Arkhangelskaia
2020-03-13 7:36 ` Alexander Turenko
[not found] ` <20200316091553.4hbyadvcubrpdzk4@tkn_work_nb>
0 siblings, 2 replies; 5+ messages in thread
From: Olga Arkhangelskaia @ 2020-03-10 11:29 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
Hi! Thanks for the review.
I have looked thought the patch and fixed it.
You can look at diffs below.
04.03.2020 16:51, Alexander Turenko пишет:
> The patch looks okay, but I would ask you to do few small modifications
> (see below).
>
> WBR, Alexander Turenko.
>
> On Tue, Feb 18, 2020 at 09:01:01PM +0300, Olga Arkhangelskaia wrote:
>> While using grep_log it is useful to have a possibility to go through crashed
> Please, keep a commit message body within 72 symbols.
[PATCH] test_run: add filename option to grep_log
To search custom logs filename option of grep_log can be used.
Usage: grep_log(self, node, waht, bytes, {filename = XXX})
Closes #196
@Changelog
Search custom log grep_log(self, node, waht, bytes, {filename = XXX})
> See https://www.tarantool.io/en/doc/1.10/dev_guide/developer_guidelines/#how-to-write-a-commit-message
>
>> instance log.
>> Usage in case of crashed instance:
>> test_run:grep_log(nil, str-to-find, nil, {path=path_to_log})
>>
>> Closes #196
>> ---
>> Issue:https://github.com/tarantool/test-run/issues/196
>> Branch:https://github.com/tarantool/test-run/pull/new/OKriw/gh-196-Extend-grep_log-for-crashed-instances
>>
>> test_run.lua | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/test_run.lua b/test_run.lua
>> index 4d8132d..68dba53 100644
>> --- a/test_run.lua
>> +++ b/test_run.lua
>> @@ -326,7 +326,9 @@ end
>> local function grep_log(self, node, what, bytes, opts)
>> local opts = opts or {}
>> local noreset = opts.noreset or false
>> - local filename = self:eval(node, "box.cfg.log")[1]
>> + -- node is used in case of alive instance, otherwise path to log can be
>> + -- used for path
>> + local filename = opts.path or self:eval(node, "box.cfg.log")[1]
> Nit: It is rule of thumb to fit comments within 66 symbols.
>
> We already use 'filename' name for this, I would use it for the option
> too.
>
> I suggest the following pattern for options:
>
> | local function f(<...>, opts)
> | local opts = opts or {}
> | local foo = opts.foo (or <default_value> if it is not nil)
> | local bar = opts.bar
> | <...the rest code...>
> | end
>
> This way it is easy to find them all in code at one glance.
diff --git a/test_run.lua b/test_run.lua
index 4d8132d..9a25246 100644
--- a/test_run.lua
+++ b/test_run.lua
@@ -326,7 +326,8 @@ end
local function grep_log(self, node, what, bytes, opts)
local opts = opts or {}
local noreset = opts.noreset or false
- local filename = self:eval(node, "box.cfg.log")[1]
+ -- if instance has crashed provide filename to use grep_log
+ local filename = opts.filename or self:eval(node, "box.cfg.log")[1]
local file = fio.open(filename, {'O_RDONLY', 'O_NONBLOCK'})
local function fail(msg)
>> local file = fio.open(filename, {'O_RDONLY', 'O_NONBLOCK'})
>>
>> local function fail(msg)
>> --
>> 2.20.1 (Apple Git-117)
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Tarantool-patches] [PATCH] test_run: add path option to grep_log
2020-03-10 11:29 ` Olga Arkhangelskaia
@ 2020-03-13 7:36 ` Alexander Turenko
[not found] ` <20200316091553.4hbyadvcubrpdzk4@tkn_work_nb>
1 sibling, 0 replies; 5+ messages in thread
From: Alexander Turenko @ 2020-03-13 7:36 UTC (permalink / raw)
To: Olga Arkhangelskaia; +Cc: tarantool-patches
Pushed to test-run with changes in the commit message (see below).
Updated the submodule in tarantool on the following branches: master,
2.3, 2.2, 1.10.
CCed Kirill.
WBR, Alexander Turenko.
> > > While using grep_log it is useful to have a possibility to go through crashed
> > Please, keep a commit message body within 72 symbols.
>
> [PATCH] test_run: add filename option to grep_log
Removed 'test_run:' prefix. It is convenient to determine a repository
in tarantool-patches mailing list, but have no much sense in the commit
itself (because it is already in test-run repository).
I suggest to use '--subject-prefix' option for 'git format-patch' when
preparing changes for a repository other then tarantool.
>
> To search custom logs filename option of grep_log can be used.
Expanded the description a bit:
| The option can be used when it is not possible to grab 'box.cfg.log'
| value from a tarantool instance: say, when the instance is stopped or
| crashed.
> Usage: grep_log(self, node, waht, bytes, {filename = XXX})
Fixed typo: waht.
I rewrote it a bit to avoid confusion with self / colon and so:
| Usage:
|
| | local test_run = require('test_run').new()
| | test_run:grep_log(node, what, bytes, {filename = <...>})
>
> Closes #196
>
> @Changelog
> Search custom log grep_log(self, node, waht, bytes, {filename = XXX})
Removed the changelog entry from the commit message. It should be in a
email (at least for tarantool), but should not be part of a commit
message.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Tarantool-patches] [PATCH] test_run: add path option to grep_log
[not found] ` <20200316091553.4hbyadvcubrpdzk4@tkn_work_nb>
@ 2020-03-16 9:18 ` Alexander Turenko
0 siblings, 0 replies; 5+ messages in thread
From: Alexander Turenko @ 2020-03-16 9:18 UTC (permalink / raw)
To: Olga Arkhangelskaia; +Cc: tarantool-patches
CCed the mail list.
----
(Sorry, forgot to send it at March, 12.)
Pushed to test-run with changes in the commit message (see below).
Updated the submodule in tarantool on the following branches: master,
2.3, 2.2 and 1.10.
CCed Kirill.
WBR, Alexander Turenko.
> > > While using grep_log it is useful to have a possibility to go through crashed
> > Please, keep a commit message body within 72 symbols.
>
> [PATCH] test_run: add filename option to grep_log
Removed 'test_run:' prefix. It is convenient to determine a repository
in tarantool-patches mailing list, but have no much sense in the commit
itself (because it is already in test-run repository).
I suggest to use '--subject-prefix' option for 'git format-patch' when
preparing changes for a repository other then tarantool.
>
> To search custom logs filename option of grep_log can be used.
Expanded the description a bit:
| The option can be used when it is not possible to grab 'box.cfg.log'
| value from a tarantool instance: say, when the instance is stopped or
| crashed.
> Usage: grep_log(self, node, waht, bytes, {filename = XXX})
Fixed typo: waht.
I rewrote it a bit to avoid confusion with self / colon and so:
| Usage:
|
| | local test_run = require('test_run').new()
| | test_run:grep_log(node, what, bytes, {filename = <...>})
>
> Closes #196
>
> @Changelog
> Search custom log grep_log(self, node, waht, bytes, {filename = XXX})
Removed from the commit message. It should be in a email (at least for
tarantool), but should not be part of a commit message.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-03-16 9:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-18 18:01 [Tarantool-patches] [PATCH] test_run: add path option to grep_log Olga Arkhangelskaia
2020-03-04 13:51 ` Alexander Turenko
2020-03-10 11:29 ` Olga Arkhangelskaia
2020-03-13 7:36 ` Alexander Turenko
[not found] ` <20200316091553.4hbyadvcubrpdzk4@tkn_work_nb>
2020-03-16 9:18 ` Alexander Turenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox