Session settings #247
Merged
Session settings #247
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oleg-jukovec
reviewed
Dec 9, 2022
DifferentialOrange
force-pushed
the
DifferentialOrange/gh-215-session-settings
branch
from
4e2f115
to
31a1af1
Compare
Use skipIfLess helpers like in other test conditions. Follows #119
In Tarantool, user space ids starts from 512. You can set arbitrary id or use autoincrement (sequence also starts from 512). Unfortunately, mixing spaces with autoincremented ids and spaces with explicit ids may cause id conflict [1]. Since there are cases when we cannot explicitly set space id (creating a space with SQL), a short range of free ids (from 512 to 515) causes problems. This patch increases range of free ids (now it's from 512 to 615) so it should be ok until [1] is resolved. 1. tarantool/tarantool#8036 Part of #215
DifferentialOrange
force-pushed
the
DifferentialOrange/gh-215-session-settings
branch
from
da30c5b
to
8ab466a
Compare
Set up package paths so queue test instances could be run with arbitrary work_dir. Part of #215
DifferentialOrange
force-pushed
the
DifferentialOrange/gh-215-session-settings
branch
3 times, most recently
from
9bf4566
to
c5cd184
Compare
DifferentialOrange
force-pushed
the
DifferentialOrange/gh-215-session-settings
branch
from
12096dc
to
404772c
Compare
Generate tmp work directories for each new Tarantool instance, if not specified. It prevents possible directory clash issues. Later `ioutil.TempDir` (deprecated since Go 1.17) must be replaced with `os.MkdirTemp` (introduced in Go 1.16 as a replacement) when we drop support of Go 1.16 an older. Part of #215
Make skip helpers public. Add more wrappers to reduce code duplication. Part of #215
DifferentialOrange
force-pushed
the
DifferentialOrange/gh-215-session-settings
branch
from
404772c
to
f8fe4ef
Compare
settings/request_test.go
Outdated
Comment on lines
615
to
74
| func TestRequestsAsync(t *testing.T) { | ||
| tests := []struct { | ||
| req tarantool.Request | ||
| async bool | ||
| }{ | ||
| {req: NewErrorMarshalingEnabledSetRequest(false), async: false}, | ||
| {req: NewErrorMarshalingEnabledGetRequest(), async: false}, | ||
| {req: NewSQLDefaultEngineSetRequest("memtx"), async: false}, | ||
| {req: NewSQLDefaultEngineGetRequest(), async: false}, | ||
| {req: NewSQLDeferForeignKeysSetRequest(false), async: false}, | ||
| {req: NewSQLDeferForeignKeysGetRequest(), async: false}, | ||
| {req: NewSQLFullColumnNamesSetRequest(false), async: false}, | ||
| {req: NewSQLFullColumnNamesGetRequest(), async: false}, | ||
| {req: NewSQLFullMetadataSetRequest(false), async: false}, | ||
| {req: NewSQLFullMetadataGetRequest(), async: false}, | ||
| {req: NewSQLParserDebugSetRequest(false), async: false}, | ||
| {req: NewSQLParserDebugGetRequest(), async: false}, | ||
| {req: NewSQLRecursiveTriggersSetRequest(false), async: false}, | ||
| {req: NewSQLRecursiveTriggersGetRequest(), async: false}, | ||
| {req: NewSQLReverseUnorderedSelectsSetRequest(false), async: false}, | ||
| {req: NewSQLReverseUnorderedSelectsGetRequest(), async: false}, | ||
| {req: NewSQLSelectDebugSetRequest(false), async: false}, | ||
| {req: NewSQLSelectDebugGetRequest(), async: false}, | ||
| {req: NewSQLVDBEDebugSetRequest(false), async: false}, | ||
| {req: NewSQLVDBEDebugGetRequest(), async: false}, | ||
| } | ||
|
|
||
| for _, test := range tests { | ||
| if async := test.req.Async(); async != test.async { | ||
| t.Errorf("An invalid async %t, expected %t", async, test.async) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Still need to add unit-tests for methods:
Code(), Body(), Context(), Ctx()
It would be good to split integration tests and unit tests into two files: tarantool_test.go? (integration), request_test.go (unit).
There was a problem hiding this comment.
Separated, added simple unit tests (without Body buffer comparison)
There was a problem hiding this comment.
without Body buffer comparison
If we want complete beauty, it is better to add (SelectRequest.Body() == GetRequest.Body(), UpdateRequest.Body() == SetRequest.Body()), but it doesn't look critical, so up to you.
DifferentialOrange
force-pushed
the
DifferentialOrange/gh-215-session-settings
branch
from
f87ee86
to
cdf2abe
Compare
oleg-jukovec
approved these changes
Dec 15, 2022
Support session settings in a separate subpackage "settings" [1]. It allows to create a specific requests to get or set session settings. Settings are independent for each connection. 1. https://www.tarantool.io/en/doc/latest/reference/reference_lua/box_space/_session_settings/ Closes #215
DifferentialOrange
force-pushed
the
DifferentialOrange/gh-215-session-settings
branch
from
cdf2abe
to
7665568
Compare
AnaNek
approved these changes
Dec 21, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Support session settings in a separate subpackage "settings" [1]. It allows to create a specific Select/Update requests to get or set session settings. Settings are independent for each connection.
I didn't forget about (remove if it is not applicable):
Closes #215