From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: Kirill Yukhin <kyukhin@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH V6 00/10] Extending error functionality
Date: Mon, 20 Apr 2020 02:26:13 +0200 [thread overview]
Message-ID: <6d673bc4-c85b-219e-6ff4-79efc7627275@tarantool.org> (raw)
In-Reply-To: <cover.1587334824.git.lvasiliev@tarantool.org>
Hi!
In short: formally LGTM.
Long version:
I don't like doing and reviewing patches in such a hurry.
This feature clearly lacked planning, design, and discussion
with community, RFC for the final version before its
implementation.
It is still unfinished because of underdesigned traceback
feature, because of payload absence. IMO, MP_EXT is also an
overkill. Tuples live fine as MP_ARRAY, and they are the most
used type. We should have gone for simple MP_MAP, without
MP_EXT. Just a map.
It is worth mentioning separately, how hard it is to use the
error marshaling now, because of this session setting. And
there still is no way to enable the feature without touching
the session, even if all my connectors support it. As I
mentioned, enabling it for every session manually is a
non-trivial task for a user.
But we have releases coming, so lets push all in whatever
state it is, of course.
next prev parent reply other threads:[~2020-04-20 0:26 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-19 22:25 Leonid Vasiliev
2020-04-19 22:25 ` [Tarantool-patches] [PATCH V6 01/10] error: add custom error type Leonid Vasiliev
2020-04-19 22:25 ` [Tarantool-patches] [PATCH V6 02/10] session: add offset to SQL session settings array Leonid Vasiliev
2020-04-19 22:25 ` [Tarantool-patches] [PATCH V6 03/10] error: add session setting for error type marshaling Leonid Vasiliev
2020-04-19 22:25 ` [Tarantool-patches] [PATCH V6 04/10] error: update constructors of some errors Leonid Vasiliev
2020-04-19 22:25 ` [Tarantool-patches] [PATCH V6 05/10] box: move Lua MP_EXT decoder from tuple.c Leonid Vasiliev
2020-04-19 22:25 ` [Tarantool-patches] [PATCH V6 06/10] error: add error MsgPack encoding Leonid Vasiliev
2020-04-19 22:25 ` [Tarantool-patches] [PATCH V6 07/10] error: export error_unref() function Leonid Vasiliev
2020-04-19 22:25 ` [Tarantool-patches] [PATCH V6 08/10] error: make iproto errors reuse mp_error module Leonid Vasiliev
2020-04-19 22:25 ` [Tarantool-patches] [PATCH V6 09/10] iproto: rename IPROTO_ERROR and IPROTO_ERROR_STACK Leonid Vasiliev
2020-04-19 22:25 ` [Tarantool-patches] [PATCH V6 10/10] error: fix iproto error stack overlapped by old error Leonid Vasiliev
2020-04-20 0:26 ` Vladislav Shpilevoy [this message]
2020-04-20 8:05 ` [Tarantool-patches] [PATCH V6 00/10] Extending error functionality lvasiliev
2020-04-20 8:05 ` Kirill Yukhin
2020-04-21 19:03 ` Konstantin Osipov
2020-04-22 16:17 ` lvasiliev
2020-04-22 17:23 ` Konstantin Osipov
2020-04-20 8:30 ` Kirill Yukhin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6d673bc4-c85b-219e-6ff4-79efc7627275@tarantool.org \
--to=v.shpilevoy@tarantool.org \
--cc=kyukhin@tarantool.org \
--cc=tarantool-patches@dev.tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH V6 00/10] Extending error functionality' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox