Re: [Tarantool-patches] [PATCH 1/2] box: make box.cfg idempotent function

[Alexander Turenko <alexander.turenko@tarantool.org>]

I updated the patchset and placed it on the
Totktonada/gh-4231-box-execute-idempotence branch. I'll send it soon.

Aside of changes around Igor's review I made the following changes:

* Rewrote w/o ffi call, using a module local variable.
* Fixed <load_cfg> to actually reconfigure box
  (reload_cfg(cfg) -> reload_cfg(box.cfg, cfg)).
* Replace box.execute with lbox_execute at end of box loading.
* Added a test for <box_load_and_execute>() and box.execute() at box
  loading (in parallel in separate fibers).
* Polished tests: remove execute_is_immutable() function, splitted
  box.execute and box.cfg tests.
* Clarified dynamic box option set functions contract.

> > +local function box_is_configured()
> > +    return ffi.C.box_is_configured()
> > +end
> > +
> 
> Minor: Since box_is_configured is introduced within this patchset it
> could be placed properly in the first patch of the series. Feel free to
> ignore.

Fixed.

> > diff --git a/test/box-tap/gh-4231-immutable-box-execute.test.lua b/test/box-tap/gh-4231-immutable-box-execute.test.lua
> 
> Minor: test chunk name is left unchanged since the previous version and
> doesn't respect the commit message wording.

Renamed to gh-4231-box-execute-idempotence.test.lua.

> > --- gh-4231: box.execute should be an idempotent function
> > --- meaning its effect should be the same if a user chooses
> > --- to use it before explicit box.cfg invocation
> > +-- gh-4231: box.execute should be an idempotent function meaning
> > +-- its effect should be the same if the user chooses to save it
> > +-- before explicit box.cfg{} invocation and use the saved version
> > +-- afterwards.
> > +-- Within the scope of the same issue box.cfg method should also
> > +-- be kept idempotent for the same reasons.
> >  --
> >  
> > -local function execute_is_immutable(execute, cmd, msg)
> > -    local status, err = pcall(execute, cmd)
> > -    test:ok(status and type(err) == 'table', msg)
> > -end
> > -
> >  local box_execute_stub = box.execute
> > --- explicit call to load_cfg
> > +local box_cfg_stub = box.cfg
> > +
> > +-- Explicit box configuration that used to change the behavior.
> >  box.cfg{}
> > +
> >  local box_execute_actual = box.execute
> > +local box_cfg_actual = box.cfg
> >  
> > -execute_is_immutable(box_execute_stub,
> > +local function is_idempotent(method, cmd, msg)
> 
> Minor: IMHO these changes are better to be moved to the first patch of
> the series. Feel free to ignore.

Fixed.