> 22 марта 2018 г., в 22:33, Konstantin Osipov написал(а): > > * Vladislav Shpilevoy > [18/03/22 22:18]: > > The patch generally looks good to me, I can only nitpick on new > things. > > How about renaming do_connect to connect_impl and wrap_connect to > wrap_socket or simply wrap, or maybe bless or imbue? Ok. > >> + if existing_connection then >> + connection = existing_connection >> + existing_connection = nil > > Why is it necessary to set it to nil? On reconnect it will call protocol_sm again, and it must no get old existing_connection again. > >> + assert(greeting) >> + assert(greeting.protocol ~= 'Lua console') > > Please keep in mind that in Lua there are no asserts - it's a > runtime check. Ok. >> + >> +local function wrap_socket(connection, greeting, url, opts) >> + if connection == nil or type(greeting) ~= 'table' then >> + error('Usage: netbox.wrap_socket(socket, greeting, [opts])') >> + end >> + if greeting.protocol == 'Lua console' then >> + error('Can not wrap console socket') >> + end >> + opts = opts or {} >> + if not opts.user and not opts.password then >> + opts.user, opts.password = url.login, url.password >> + end >> + return do_connect(url.host, url.service, opts, connection, greeting) >> +end > > Looks like you should make the next step, i.e. take the piece of > do_connect which establishes a connection and move it to > connect(), and then you can the remains of do_connect into wrap() > and call wrap() connect(). > > Did you try to do it? Had any trouble? Connection establishment must be in connect inside create_transport, because even if an existing socket was wrapped, it still can have opts.reconnect_after option and the following scenario: socket = ... con = netbox.wrap_socket(socket, ..., {reconnect_after = ...}) -- -- use connection ... -- Suppose it is broken due to error. Then netbox will try to establish a new connection after 'reconnect_after' seconds. > > -- > Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 > http://tarantool.org - www.twitter.com/kostja_osipov