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.

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

Fix/issue 33 #34

Closed
wants to merge 6 commits into from
Closed

Conversation

tarun-kavipurapu
Copy link
Contributor

Fixes #33

Copy link
Contributor

@lucifercr07 lucifercr07 left a comment

Choose a reason for hiding this comment

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

@tarun-kavipurapu thanks for the fix, can you please add some tests around this.

@lucifercr07
Copy link
Contributor

@tarun-kavipurapu can you please check for MGET commands response as part of this PR and fix it.

@lucifercr07
Copy link
Contributor

@tarun-kavipurapu please resolve the conflicts.

@tarun-kavipurapu tarun-kavipurapu marked this pull request as draft October 8, 2024 09:32
@tarun-kavipurapu tarun-kavipurapu marked this pull request as ready for review October 8, 2024 09:32

// command2callback map
var command2callback = map[string]DiceDBCallback{
"AUTH": renderSimpleString,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make this other way around? Like group methods to commands list?
It'd reduce redundancy.
Something like below and add a finder on top of this

// Create hashsets (maps) for each command group
var simpleStringCommands = map[string]struct{}{
	"AUTH":    {}, "PING":    {}, "SELECT":  {},
	"SET":     {}, "RESTORE": {}, "FLUSHDB": {},
}

strItem := fmt.Sprintf("%v", item)

// Properly format with double quotes
quoted := fmt.Sprintf("\"%q\"", strItem)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can it just be single quotes? We're printing with 2 quotes? Is behaviour same as CLI?

dice > KEYS k*
1) ""k""
2) ""k1""

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add tests.

@lucifercr07
Copy link
Contributor

Closing, as not required after recent SDK changes. Thanks a lot for the efforts @tarun-kavipurapu .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request Parsing Fix
2 participants