Add public API with request object types #162

oleg-jukovec · 2022-04-13T07:25:13Z

This patch provides request types for part of space operations: Select, Update
and Upstream. It allows to create requests step by step. The main idea here
is too provide more extensible approach to create requests.

Part of #126

CHANGELOG.md

client_tools.go

ligurio removed their request for review

vr009

The patch looks good to me. Please, consider my comments.

Also there is a kind of a "rule" to add some prefix word in the commit header, like "api: add a request object for select/update/upsert". See CONTRIBUTING.md for details.

What about the implementation of request object, and it's filling with chaining methods, I like this approach. It is more comfortable, to use the constructor and other methods instead of filling the structure manually and passing it.

future.go

tarantool_test.go

CHANGELOG.md

const.go

oleg-jukovec · 2022-06-02T09:13:59Z

I've updated the pull request. Changes:

add request object API for all queries
add Do/DoTyped/DoAsync to ConnectionPool and ConnectionMulti types
fix some comments

In general it's ready, but in additional, we can use Request objects in the internal implementation instead of closures:

go-tarantool/request.go

Lines 272 to 275 in d44ffa0

    
           return future.send(conn, func(enc *msgpack.Encoder) error { 
        
           	enc.EncodeMapLen(2) 
        
           	return future.fillInsert(enc, spaceNo, tuple) 
        
           })

It will simplify the Request interface:

type Request interface {
	Code() int32
	BodyFunc(resolver SchemaResolver) (func(enc *msgpack.Encoder) error, error)
}

There will be no need to return a closure from BodyFunc and it can be replaced by just Body(enc *msgpack.Encoder). I like this change and I want to do this, but I would like to get some feedback first.

Totktonada

I think it is the nice addition!

Looks okay for me at brief glance.

request.go

tarantool_test.go

request.go

Totktonada · 2022-06-09T22:35:58Z

I like this change and I want to do this, but I would like to get some feedback first.

I don't see any downsides in the proposed refactoring.

oleg-jukovec · 2022-06-14T16:34:13Z

I've updated the pull request. Changes:

Commits with request objects (Select/Update/Upsert and Insert/.../Ping) have been squashed into one.
Added a separate commit with refactoring. Request objects are used by an internal implementation instead of closures.

I don't see what else can be changed anymore.

DifferentialOrange

I've reviewed some parts of this review and will review the remaining parts tomorrow. But it seems like a really useful PR.

connection.go

CHANGELOG.md

DifferentialOrange

I couldn't say I dig too deep, but it seems LGTM

request.go

example_test.go

connection.go

Totktonada · 2022-06-18T12:13:33Z

@oleg-jukovec Are you plan to finish any work here or it is time to proceed?

oleg-jukovec · 2022-06-18T18:42:01Z

@oleg-jukovec Are you plan to finish any work here or it is time to proceed?

I plan to merge it after #171 .

This patch provides request types. It allows to create requests step by step. The main idea here is too provide more extensible approach to create requests. It renames IPROTO_* constants that identify requests from `<Name>Request` to `<Name>RequestCode` to provide names for request types. Part of #126

This patch provides Do, DoTyped and DoAsync functions for ConnectionPool and ConnectionMulti types. Part of #126

The patch is a refactoring of an internal logic. It replaces the usage of closures to request objects to construct a request body. After the patch all Connection.* requests use request objects inside. Closes #126

oleg-jukovec requested a review from Totktonada April 13, 2022 07:26

oleg-jukovec changed the title ~~Add public API with a request object for Select/Update/Upstream~~ Add public API with a request object for Select/Update/Upsert Apr 15, 2022

oleg-jukovec force-pushed the oleg-jukovec/gh-126-request-object-select branch 2 times, most recently from 08d91c5 to fe3d5e3 Compare April 20, 2022 12:49

ligurio previously requested changes Apr 21, 2022

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

client_tools.go Outdated Show resolved Hide resolved

oleg-jukovec force-pushed the oleg-jukovec/gh-126-request-object-select branch 2 times, most recently from 3ce324b to 263fb81 Compare April 22, 2022 06:08

oleg-jukovec requested a review from ligurio April 22, 2022 06:11

oleg-jukovec force-pushed the oleg-jukovec/gh-126-request-object-select branch 3 times, most recently from ea269e8 to 3f58815 Compare April 29, 2022 12:04

Totktonada requested a review from DifferentialOrange May 12, 2022 12:52

ligurio removed their request for review May 16, 2022 09:28

oleg-jukovec force-pushed the oleg-jukovec/gh-126-request-object-select branch 2 times, most recently from 183da38 to 017eb2a Compare May 19, 2022 15:22

Totktonada requested a review from vr009 May 20, 2022 10:45

oleg-jukovec force-pushed the oleg-jukovec/gh-126-request-object-select branch from 017eb2a to 886b40f Compare May 25, 2022 06:26

vr009 approved these changes May 25, 2022

View reviewed changes

future.go Outdated Show resolved Hide resolved

tarantool_test.go Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

const.go Show resolved Hide resolved

oleg-jukovec force-pushed the oleg-jukovec/gh-126-request-object-select branch 2 times, most recently from 192d807 to 3818833 Compare June 1, 2022 08:31

oleg-jukovec force-pushed the oleg-jukovec/gh-126-request-object-select branch from 0d42a5e to ceb6678 Compare June 2, 2022 08:59

oleg-jukovec changed the title ~~Add public API with a request object for Select/Update/Upsert~~ Add public API with request object types Jun 2, 2022

oleg-jukovec force-pushed the oleg-jukovec/gh-126-request-object-select branch from ceb6678 to 80fe1fd Compare June 2, 2022 12:43

Totktonada approved these changes Jun 9, 2022

View reviewed changes

request.go Outdated Show resolved Hide resolved

request.go Outdated Show resolved Hide resolved

tarantool_test.go Show resolved Hide resolved

request.go Show resolved Hide resolved

oleg-jukovec force-pushed the oleg-jukovec/gh-126-request-object-select branch 2 times, most recently from 2202126 to daae235 Compare June 14, 2022 15:54

DifferentialOrange reviewed Jun 16, 2022

View reviewed changes

connection.go Show resolved Hide resolved

connection.go Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

oleg-jukovec force-pushed the oleg-jukovec/gh-126-request-object-select branch 2 times, most recently from 200ba67 to 2d477f4 Compare June 17, 2022 12:13

DifferentialOrange approved these changes Jun 17, 2022

View reviewed changes

request.go Outdated Show resolved Hide resolved

example_test.go Show resolved Hide resolved

connection.go Show resolved Hide resolved

DifferentialOrange mentioned this pull request Jun 17, 2022

sql: support prepared statements #180

Merged

3 tasks

oleg-jukovec force-pushed the oleg-jukovec/gh-126-request-object-select branch from 2d477f4 to 0343412 Compare June 20, 2022 12:50

oleg-jukovec added 3 commits June 20, 2022 16:35

api: add Do* functions to ConnectionPool and ConnectionMulti

56d16e9

This patch provides Do, DoTyped and DoAsync functions for ConnectionPool and ConnectionMulti types. Part of #126

code health: all requests use request objects

e62bd36

The patch is a refactoring of an internal logic. It replaces the usage of closures to request objects to construct a request body. After the patch all Connection.* requests use request objects inside. Closes #126

oleg-jukovec force-pushed the oleg-jukovec/gh-126-request-object-select branch from 0343412 to e62bd36 Compare June 20, 2022 13:36

oleg-jukovec merged commit 7116a33 into master Jun 20, 2022

oleg-jukovec deleted the oleg-jukovec/gh-126-request-object-select branch June 20, 2022 13:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add public API with request object types #162

Add public API with request object types #162

oleg-jukovec commented Apr 13, 2022

vr009 left a comment

oleg-jukovec commented Jun 2, 2022 •

edited

Loading

Totktonada left a comment

Totktonada commented Jun 9, 2022

oleg-jukovec commented Jun 14, 2022 •

edited

Loading

DifferentialOrange left a comment

DifferentialOrange left a comment

Totktonada commented Jun 18, 2022

oleg-jukovec commented Jun 18, 2022

Add public API with request object types #162

Add public API with request object types #162

Conversation

oleg-jukovec commented Apr 13, 2022

vr009 left a comment

Choose a reason for hiding this comment

oleg-jukovec commented Jun 2, 2022 • edited Loading

Totktonada left a comment

Choose a reason for hiding this comment

Totktonada commented Jun 9, 2022

oleg-jukovec commented Jun 14, 2022 • edited Loading

DifferentialOrange left a comment

Choose a reason for hiding this comment

DifferentialOrange left a comment

Choose a reason for hiding this comment

Totktonada commented Jun 18, 2022

oleg-jukovec commented Jun 18, 2022

oleg-jukovec commented Jun 2, 2022 •

edited

Loading

oleg-jukovec commented Jun 14, 2022 •

edited

Loading