-
Notifications
You must be signed in to change notification settings - Fork 60
api: fix splice update operation #353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
api: fix splice update operation #353
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add examples of using the type Operations and []Op{} as update arguments.
(I see ExampleUpdateRequest, but that's not quite it).
15cb5f6
to
ab4af57
Compare
test_helpers/utils.go
Outdated
@@ -200,6 +200,14 @@ func SkipIfWatchOnceUnsupported(t *testing.T) { | |||
SkipIfFeatureUnsupported(t, "watch once", 3, 0, 0) | |||
} | |||
|
|||
// SkipIfCrudSpliceUnsupported skips test run if Tarantool without support of | |||
// crud splice update operation is used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crud splice is core Tarantool splice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But for some reason, using splice in crud is not supported for Tarantool < 2.0. While we can use splice from the core.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you show the exact error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. For example:
https://github.com/tarantool/go-tarantool/actions/runs/6875782438/job/18700075402
--- FAIL: TestCrudProcessData/Update (0.00s)
tarantool_test.go:583: Failed to perform CRUD request on CRUD side: map[interface {}]interface {}{"class_name":"UpdateError", "err":"Failed to update: Unknown UPDATE operation", "file":"...ool/crud/testdata/.rocks/share/tarantool/crud/update.lua", "line":0xb0, "str":"UpdateError: Failed to update: Unknown UPDATE operation"}
But for the core:
PASS
ok github.com/tarantool/go-tarantool/v2 4.867s
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that this one is more "broken" than "unsupported": tarantool/crud#397
Thank for the report. I think it's worth to add a link to the issue to skip helper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated function name and added a link to an issue.
ab4af57
to
fa47071
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the patch! Just few notes.
5c6ff3a
to
00fa43c
Compare
00fa43c
to
3a1a237
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the fix! No objections from my side after resolving the last discussion.
Splice update operation (`:`) accepts 5 and only 5 arguments. It was parsed and encoded incorrectly with only 3 argument (as every other update operations). Also fixed the same operation for the crud. Part of #348
`Op` struct made private. Change all `Upsert` and `Update` requests to accept `*tarantool.Operations` as `ops` parameters instead of `interface{}`. Closes #348
3a1a237
to
56b3c82
Compare
Splice update operation (
:
) accepts 5 and only 5 arguments. It was parsed and encoded incorrectly with only 3 argument (as every other update operations).Also fixed the same operation for the crud.
Op
struct made private.Change all
Upsert
andUpdate
requests to accept*tarantool.Operations
asops
parameters instead of theinterface{}
.I didn't forget about (remove if it is not applicable):
Closes #348