On 23/09/2019 03:47, Alexander Turenko wrote:
> The patchset looks good for me.
>
> However there is one open question: a name of the new
> <serializer>.cfg({}) option. I would let Vlad decide about that (don't
> see a reason to discuss it more then we did) and then merge the
> patchset.
>
> Vlad, please, CC Kirill Yu. when you'll dedice about the option name.
>
> WBR, Alexander Turenko.
>
Thanks for the review!
Kill, I fixed the name, and all other comments. The branch is
force pushed.
> On Mon, Sep 09, 2019 at 09:00:06PM +0200, Vladislav Shpilevoy wrote:
>> The patchset fixes several problems about JSON and msgpack serializers.
>>
>> 1) msgpackffi didn't use msgpack serializer options;
>> 2) tuple serializer either;
>> 3) update of a serializer option was not reflected in its Lua representation;
>> 4) during serialization too nested tables are silently cropped and there was no
>> way to prevent it.
>>
>> Also it was discovered, that msgpackffi does not care *all* options. Not only
>> about max_depth. I am not sure if it is worth fixing here (or at all) though.
>>
>> Changes in v2:
>> - Renames;
>> - Default value a flag for whether we need to raise an error on a too nested msgpack (was crop, now error).
>>
>> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4434-box-lua-msgpack-cfg
>> Issue: https://github.com/tarantool/tarantool/issues/4434
>>
>> Vladislav Shpilevoy (4):
>> app: serializers update now is reflected in Lua
>> msgpack: make msgpackffi use encode_max_depth option
>> tuple: use global msgpack serializer in Lua tuple
>> app: allow to raise an error on too nested tables
>>
>> src/box/lua/tuple.c | 33 +++++++++++++-----
>> src/lua/msgpack.c | 4 +++
>> src/lua/msgpackffi.lua | 6 ++--
>> src/lua/utils.c | 47 +++++++++++++++----------
>> src/lua/utils.h | 32 +++++++++++++++++
>> test/app-tap/json.test.lua | 3 +-
>> test/app-tap/lua/serializer_test.lua | 31 +++++++++++++++++
>> test/app-tap/msgpack.test.lua | 3 +-
>> test/app-tap/msgpackffi.test.lua | 31 ++++++++++++++++-
>> test/box/tuple.result | 52 ++++++++++++++++++++++++++++
>> test/box/tuple.test.lua | 23 ++++++++++++
>> test/unit/luaT_tuple_new.c | 2 +-
>> third_party/lua-cjson/lua_cjson.c | 10 ++++--
>> 13 files changed, 243 insertions(+), 34 deletions(-)
>>
>> --
>> 2.20.1 (Apple Git-117)
>>