[tarantool-patches] Re: [PATCH] net.box: ignore absence of _vcollation
Alexander Turenko
alexander.turenko at tarantool.org
Wed Jul 31 01:35:15 MSK 2019
Kirill, I have acquired LGTMs from Kostja and Vlad (with the tiny comment
below, which I had fixed). Can you proceed with the change?
https://github.com/tarantool/tarantool/issues/4307
https://github.com/tarantool/tarantool/tree/Totktonada/gh-4307-net-box-fix-schema-fetch
WBR, Alexander Turenko.
> > diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
> > index 2892bb3d3..31a8c16b7 100644
> > --- a/src/box/lua/net_box.lua
> > +++ b/src/box/lua/net_box.lua
> > @@ -754,6 +768,8 @@ local function create_transport(host, port, user, password, callback,
> > if err then return error_sm(err, hdr) end
> > dispatch_response_iproto(hdr, body_rpos, body_end)
> > local id = hdr[IPROTO_SYNC_KEY]
> > + -- trick: omit check for peer_has_vcollation: id is
> > + -- not nil
>
> Nit: usually we start comments with a capital letter and finish
> with a dot. Here and in other hunks. Other than that the patch
> LGTM.
I'm aware of that. I gave glance to comments around and they are
formatted from a small letter and w/o a period, so I formatted the new
code in the same way to make reading more comfortable. Now hovewer I see
that there is no consistency in this point across net_box.lua file. So I
agree with you: it worth to write new comments in our standard way.
Applied an incremental diff at end of the email.
I have applied the diff, rebased the branch upward current master and
force-pushed the branch.
> > +if _TARANTOOL >= '2.2.1' then \
> > + return parts[1].collation == 'test' \
> > +else \
> > + return parts[1].collation_id == collation_id \
>
> I like this new test-run feature with '\', looks way better
> than setopt delimited, and looks like C.
Thanks!
----
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 31a8c16b7..ea169637d 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -743,7 +743,7 @@ local function create_transport(host, port, user, password, callback,
set_state('active')
return iproto_sm(schema_version)
end
- -- _vcollation view was added in 2.2.0-389-g3e3ef182f
+ -- _vcollation view was added in 2.2.0-389-g3e3ef182f.
local peer_has_vcollation = version_at_least(greeting.version_id,
2, 2, 1)
local select1_id = new_request_id()
@@ -768,8 +768,8 @@ local function create_transport(host, port, user, password, callback,
if err then return error_sm(err, hdr) end
dispatch_response_iproto(hdr, body_rpos, body_end)
local id = hdr[IPROTO_SYNC_KEY]
- -- trick: omit check for peer_has_vcollation: id is
- -- not nil
+ -- Trick: omit check for peer_has_vcollation: id is
+ -- not nil.
if id == select1_id or id == select2_id or id == select3_id then
-- response to a schema query we've submitted
local status = hdr[IPROTO_STATUS_KEY]
@@ -791,7 +791,7 @@ local function create_transport(host, port, user, password, callback,
end
until response[select1_id] and response[select2_id] and
(not peer_has_vcollation or response[select3_id])
- -- trick: response[select3_id] is nil when the key is nil
+ -- Trick: response[select3_id] is nil when the key is nil.
callback('did_fetch_schema', schema_version, response[select1_id],
response[select2_id], response[select3_id])
set_state('active')
@@ -1286,8 +1286,8 @@ function remote_methods:_install_schema(schema_version, spaces, indices,
local pkcollationid = index[PARTS][k].collation
local pktype = index[PARTS][k][2] or index[PARTS][k].type
local pkfield = index[PARTS][k][1] or index[PARTS][k].field
- -- resolve a collation name if a peer has
- -- _vcollation view
+ -- Resolve a collation name if a peer has
+ -- _vcollation view.
local pkcollation = nil
if pkcollationid ~= nil and collations ~= nil then
pkcollation = collations[pkcollationid + 1][2]
More information about the Tarantool-patches
mailing list