Skip to content
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.

Sign up for GitHub

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

Jump to bottom

GO Implement Dump, Restore and ObjectEncoding Command #2781

Merged
merged 29 commits into from
Jan 17, 2025
Merged

GO Implement Dump, Restore and ObjectEncoding Command #2781

merged 29 commits into from
Jan 17, 2025

Conversation

EdricCua
Copy link
Collaborator

@EdricCua EdricCua commented Dec 3, 2024 •
edited
Loading

Issue link

This Pull Request is linked to issue (URL): https://github.com/orgs/valkey-io/projects/10/views/10?pane=issue&itemId=91968108&issue=valkey-io%7Cvalkey-glide%7C2853

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one issue.
  • Commit message has a detailed description of what changed and why.
  • Tests are added or updated.
  • CHANGELOG.md and documentation files are updated.
  • Destination branch is correct - main or release
  • Commits will be squashed upon merging.

@EdricCua EdricCua requested a review from a team as a code owner December 3, 2024 09:23
@Yury-Fridlyand Yury-Fridlyand added the go golang wrapper label Dec 3, 2024
Yury-Fridlyand
Yury-Fridlyand previously requested changes Dec 3, 2024
View reviewed changes
Copy link
Collaborator

@Yury-Fridlyand Yury-Fridlyand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend implementing dump together with restore so you can test it and validate that both work. Otherwise dump is untested.

go/api/generic_commands.go Outdated Show resolved Hide resolved
@EdricCua EdricCua changed the title GO Implement Dump and ObjectEncoding Command GO Implement Dump, Restore and ObjectEncoding Command Dec 5, 2024
@EdricCua
Add restore command
7b4710b 
Signed-off-by: EdricCua <[email protected]>
yipin-chen
yipin-chen reviewed Dec 5, 2024
View reviewed changes
go/api/base_client.go Outdated Show resolved Hide resolved
@Yury-Fridlyand Yury-Fridlyand self-requested a review December 6, 2024 18:21
@Yury-Fridlyand Yury-Fridlyand dismissed their stale review December 6, 2024 18:22

Restore added

Yury-Fridlyand
Yury-Fridlyand reviewed Dec 27, 2024
View reviewed changes
Copy link
Collaborator

@Yury-Fridlyand Yury-Fridlyand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few small comments left.
Since dump-restore cycle works fine we can confirm that go client support binary strings. That's really good news.

go/api/command_options.go Outdated Show resolved Hide resolved
go/api/base_client.go Outdated Show resolved Hide resolved
go/api/generic_commands.go Outdated Show resolved Hide resolved
go/api/generic_commands.go Outdated Show resolved Hide resolved
go/api/generic_commands.go Outdated Show resolved Hide resolved
go/integTest/shared_commands_test.go Outdated Show resolved Hide resolved
@prateek-kumar-improving
Copy link
Collaborator

Please update PR description checklist and add the linked issue for this PR.

@prateek-kumar-improving
Copy link
Collaborator

Please update changelog.md file.

Yury-Fridlyand
Yury-Fridlyand approved these changes Jan 8, 2025
View reviewed changes
Copy link
Collaborator

@Yury-Fridlyand Yury-Fridlyand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but CI is red

go/integTest/shared_commands_test.go Outdated Show resolved Hide resolved
@EdricCua
Copy link
Collaborator Author

EdricCua commented Jan 9, 2025

Hi @Yury-Fridlyand - Test failing in json_module_test.go and vss_module_test.go

@EdricCua
Fix merge conflict
82345bb 
Signed-off-by: EdricCua <[email protected]>
@Yury-Fridlyand
Copy link
Collaborator

Yury-Fridlyand commented Jan 10, 2025 •
edited
Loading

on redis 6

        	Error Trace:	/home/runner/work/valkey-glide/valkey-glide/go/integTest/shared_commands_test.go:4507
        	            				/home/runner/work/valkey-glide/valkey-glide/go/integTest/glide_test_suite_test.go:269
        	Error:      	Not equal: 
        	            	expected: "listpack"
        	            	actual  : "quicklist"

@Yury-Fridlyand Yury-Fridlyand merged commit b1a2c1f into valkey-io:main Jan 17, 2025
18 checks passed
Maayanshani25 pushed a commit to Maayanshani25/valkey-glide that referenced this pull request Jan 19, 2025
@EdricCua @Maayanshani25
GO Implement Dump, Restore and ObjectEncoding Command (valkey-io#2781)
f1ceffb 
* GO Implement Dump and ObjectEncoding command

Signed-off-by: EdricCua <[email protected]>
edlng pushed a commit to Bit-Quill/valkey-glide that referenced this pull request Jan 20, 2025
@EdricCua @edlng
GO Implement Dump, Restore and ObjectEncoding Command (valkey-io#2781)
d8ae33d 
* GO Implement Dump and ObjectEncoding command

Signed-off-by: EdricCua <[email protected]>
Signed-off-by: Edward Liang <[email protected]>
eifrah-aws pushed a commit to eifrah-aws/glide-for-redis that referenced this pull request Jan 23, 2025
@EdricCua @eifrah-aws
GO Implement Dump, Restore and ObjectEncoding Command (valkey-io#2781)
ecd2b88 
* GO Implement Dump and ObjectEncoding command

Signed-off-by: EdricCua <[email protected]>
barshaul pushed a commit to barshaul/glide-for-redis that referenced this pull request Jan 28, 2025
@EdricCua @barshaul
GO Implement Dump, Restore and ObjectEncoding Command (valkey-io#2781)
6a06cc5 
* GO Implement Dump and ObjectEncoding command

Signed-off-by: EdricCua <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yipin-chen yipin-chen yipin-chen left review comments

@tjzhang-BQ tjzhang-BQ tjzhang-BQ left review comments

@Yury-Fridlyand Yury-Fridlyand Yury-Fridlyand approved these changes

@prateek-kumar-improving prateek-kumar-improving prateek-kumar-improving approved these changes

Assignees
No one assigned
Labels
go golang wrapper
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@EdricCua @prateek-kumar-improving @Yury-Fridlyand @tjzhang-BQ @yipin-chen