From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 52BDF4696C3 for ; Mon, 20 Apr 2020 03:26:15 +0300 (MSK) References: From: Vladislav Shpilevoy Message-ID: <6d673bc4-c85b-219e-6ff4-79efc7627275@tarantool.org> Date: Mon, 20 Apr 2020 02:26:13 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH V6 00/10] Extending error functionality List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kirill Yukhin Cc: tarantool-patches@dev.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.