From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 044BC4696C3 for ; Sun, 22 Mar 2020 22:31:56 +0300 (MSK) References: <20200317172109.33111-1-k.sosnin@tarantool.org> From: Vladislav Shpilevoy Message-ID: <2d332fe0-5c65-a791-8489-86564d70819f@tarantool.org> Date: Sun, 22 Mar 2020 20:31:54 +0100 MIME-Version: 1.0 In-Reply-To: <20200317172109.33111-1-k.sosnin@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH] box: provide a user friendly frontend for accessing session settings List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chris Sosnin , korablev@tarantool.org, tarantool-patches@dev.tarantool.org Hi! Is this patch in scope of the other patches of the same thread? If it is, it should be sent together with them. Otherwise I don't see what is going to be pushed if I will give ok to the patch. I see that at least sql patch is changed, but it not resent. See 3 comments below. On 17/03/2020 18:21, Chris Sosnin wrote: > - space_object:update() is hard to use for configuring session settings, > so we provide box.session.setting table, which can be used in a much more > native way. > > - Prior to this patch sql settings were not accessible before box.cfg() > call, even though these flags can be set right after session creation. > > Part of #4711 > --- > This patch provides the following syntax as was suggested by Nikita: > box.session.settings. = . Doc request in sql-patch is 1. In the commit message you said 'box.session.setting'. Not 'settings'. What option did you want to use? > updated too. > > Note, that I had to break the null - error message rule as long as > __newindex doesn't return anything, the only way to tell the user > about the error is to raise it. > > I moved it to the new branch: > branch:https://github.com/tarantool/tarantool/tree/ksosnin/gh-4712-session-settings-v2 > issuse:https://github.com/tarantool/tarantool/issues/4711 > > diff --git a/src/box/lua/session.c b/src/box/lua/session.c > index c6a600f6f..66d59c8c7 100644 > --- a/src/box/lua/session.c > +++ b/src/box/lua/session.c > @@ -411,6 +413,116 @@ lbox_session_on_access_denied(struct lua_State *L) > lbox_push_on_access_denied_event, NULL); > } > > +static int > +lbox_session_setting_get_by_id(struct lua_State *L) 2. This function is never called from Lua. You don't need to pass ID via Lua stack. Add it as a C argument. > +{ > + assert(lua_type(L, -1) == LUA_TNUMBER); > + int sid = lua_tointeger(L, -1); > + lua_pop(L, 1); > + const char *mp_pair, *mp_pair_end; > + session_settings[sid].get(sid, &mp_pair, &mp_pair_end); > + uint32_t len; > + mp_decode_array(&mp_pair); > + mp_decode_str(&mp_pair, &len); > + enum field_type field_type = session_settings[sid].field_type; > + if (field_type == FIELD_TYPE_BOOLEAN) { > + bool value = mp_decode_bool(&mp_pair); > + lua_pushboolean(L, value); > + } else { > + assert(field_type == FIELD_TYPE_STRING); > + const char *str = mp_decode_str(&mp_pair, &len); > + lua_pushlstring(L, str, len); > + } > + return 1; > +} > + > +static int > +lbox_session_setting_get(struct lua_State *L) { 3. Please, use a separate line for {. The same for serialize.