Re: [Tarantool-patches] [PATCH 3/4] box: provide a user friendly frontend for accessing session settings

[Nikita Pettik <korablev@tarantool.org>]

On 16 Mar 23:53, Vladislav Shpilevoy wrote:
> On 16/03/2020 17:08, Nikita Pettik wrote:
> > On 17 Feb 15:12, 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
> >> ---
> > 
> > tarantool> box.session.settings.sql_vdbe_debug
> > ---
> > - false
> > ...
> > 
> > tarantool> box.session.settings.sql_vdbe_debug = true
> > ---
> > ...
> > 
> > tarantool> box.session.settings.sql_vdbe_debug
> > ---
> > - true
> > ...
> 
> Yeah, we can ban this. To avoid confusion.
> 
> > 
> > tarantool> box.execute("select 1")
> > ---
> > - metadata:
> >   - name: '1'
> >     type: integer
> >   rows:
> >   - [1]
> > ...
> > 
> > Looks inconsistent. Can we use instead of :set() method simple
> > table value assignment? Otherwise accessing row table values
> 
> Assignment would require not to store settings in box.session.settings,
> to be able to redefine __newindex metamethod. If we don't store them, we
> kill autocompletion, which was asked explicitly by somebody.
> 
> But I am on your side here - I don't think autocompletion worth this
> complication. Who wants to look at existing settings can just print
> box.sessions.settings table.
> 
> > should be disallowed. Same concerns :get() method. Why ever
> > anyone should bother with :get() when one can access table value
> > via simple indexing?
> 
> Hm. But there is no :get() method. We didn't implement getting, because
> no one asked for this.

As far as I remember no one either asked for :set() method (except for me) :)

> And indeed, usually you just set settings, without
> checking if set really worked. I was thinking we could introduce get if
> someone asks for that. But up to you. Can be added now as well.

I do not insist since do not consider this to be prio1 task (as well
as any other issue connected with settings machinery).