Compatibility with rejson:latest 2.0.6 by breno12321 · Pull Request #60 · nitishm/go-rejson

breno12321 · 2022-01-15T03:59:26Z

Problem

Problems with the JSON.DEBUG
For some reason redigo is returning JSON.TYPE string as a Byte array at version >2.0.6

Solution

JSON.DEBUG instability may be a reason to disable their tests for now or make it more dynamic for more expected values
For the redigo JSON.TYPE a type handling could solve the issue

Shivam010

I haven't really used debug command much, I want to check it first before I comment anything on it.

And thanks @breno12321 for your contribution.

Shivam010 · 2022-01-15T09:14:13Z

+	res, command_err := r.Conn.Do(name, args...)
+
+	if err != nil {
+		return nil, command_err
+	}


Why use the command_err variable but not check it? Why not use err variable itself here ?

it said that no variable on the left were created, something like this in my IDE I don't know. I can try again later today

I think maybe its due to the err declaration above 🤔

func (r *Redigo) JSONType(key, path string) (res interface{}, err error) { name, args, err := rjs.CommandBuilder(rjs.ReJSONCommandTYPE, key, path) if err != nil { return nil, err } res, err = r.Conn.Do(name, args...) if err != nil { return nil, err } switch v := res.(type) { case string: return v, nil case []byte: return string(v), nil case nil: return default: err := fmt.Errorf("type returned not expected %T", v) return nil, err } }

Changing to just = instead of := it works, sorry, getting used to Go notation hehe

Newbie question ❓ @Shivam010 this way the res is already created by the returning definition for the function? 🤔 so no need to var assignment?

Yes. var and := declare a new variable and hence, I think you were getting the error. = this will just assign the value to already created variables

Shivam010 · 2022-01-15T09:17:54Z

+	switch v := res.(type) {
+	case string:
+		return v, nil
+	case []uint8:
+		return string(v), nil
+	case nil:
+		return
+	default:
+		err := fmt.Errorf("type returned not expected %T", v)
+		return nil, err
+	}


Yes, I agree with this. Just a few suggestions, it would be better if we directly return the value in case of default and we should also handle the []byte type.

And I don't think nil check should be required, is there something I am missing here?

There is some tests regarding nil inputs and they would break if not handled properly

https://github.com/nitishm/go-rejson/blob/master/rejson_test.go#L763-L770

About the byte agree ill make the change, thought []uint8 would cover

@Shivam010 👀

Maybe I leave just the []byte, more straight forward 😅

Yes, change it to []byte, that would be more clear

breno12321 · 2022-01-15T12:59:23Z

I thank U @Shivam010 for helping me, I started to learn Go this week so.... All help is welcome and tips and tricks and good resources haha

Shivam010 · 2022-01-18T14:00:05Z

@breno12321 It's my pleasure to help you.

You can also join the Gophers slack community https://invite.slack.gobridge.org/ there's a lot of great people out there.

And here are the resources for you (quote from Slack):

gopherAPP  07:28 PM
If you'd like to join others who are new to Go, we have the #newbies channel.
Here are some resources you should check out if you are learning / new to Go:
First you should take the language tour: https://tour.golang.org/
Then, you should visit:
- https://golang.org/doc/code.html to learn how to organize your Go workspace
- https://golang.org/doc/effective_go.html be more effective at writing Go
- https://golang.org/ref/spec learn more about the language itself
- https://golang.org/doc/#articles a lot more reading material
There are some awesome websites as well:
- https://blog.gopheracademy.com great resources for Gophers in general
- http://gotime.fm awesome weekly podcast of Go awesomeness
- https://gobyexample.com examples of how to do things in Go
- http://go-database-sql.org how to use SQL databases in Go
- https://dmitri.shuralyov.com/idiomatic-go tips on how to write more idiomatic Go code
- https://divan.github.io/posts/avoid_gotchas will help you avoid gotchas in Go
- https://golangbot.com tutorials to help you get started in Go
- https://tutorialedge.net a collection of articles around various aspects of Go
There's also an exhaustive list of videos http://gophervids.appspot.com related to Go from various authors.
If you prefer books, you can try these:
- http://www.golangbootcamp.com/book
- http://gopl.io/
- https://www.manning.com/books/go-in-action (if you e-mail @wkennedy at bill@ardanlabs.com you can get a free copy for being part of this Slack)
If you want to learn how to organize your Go project, make sure to read: https://medium.com/@benbjohnson/standard-package-layout-7cdbc8391fc1#.ds38va3pp.
Once you are accustomed to the language and syntax, you can read this series of articles for a walkthrough the various standard library packages: https://medium.com/go-walkthrough.
Finally, https://github.com/golang/go/wiki#learning-more-about-go will give a list of even more resources to learn Go

Shivam010

Push your changes, and I think then we will be good to go

breno12321 · 2022-01-18T15:35:51Z

Question @Shivam010 besides Linting, should I remove the Debug command? Since its not consistent with the tests? The []byte I've already made the fix

Shivam010

Yes, you are right.

JSON.DEBUG seems to have some issue and the redis/rejson team is working on it.

Ref:

So, I think, for now, we can skip the debug command test cases with t.SkipNow() instead of removing them.

@breno12321 Can you add it?

breno12321 · 2022-01-27T11:23:15Z

I can skip sorry for the late response life been crazy 😅

breno12321 · 2022-01-27T11:36:13Z

Think its all good @Shivam010! 😊

Shivam010

LGTM

Shivam010 · 2022-01-27T20:11:57Z

I can skip sorry for the late response life been crazy 😅

Yeah, no problem. And thanks for your contribution.

breno12321 added 2 commits January 15, 2022 00:58

Handle Redigo byte array response for type

d4fc434

Notation _testJSONDEBUG unused for now

367fd48

Shivam010 requested changes Jan 15, 2022

View reviewed changes

breno12321 requested a review from Shivam010 January 15, 2022 14:12

Shivam010 requested changes Jan 18, 2022

View reviewed changes

breno12321 marked this pull request as ready for review January 18, 2022 23:56

Remove JSONTYPE tests and refactor assignment

175cfe9

breno12321 requested a review from Shivam010 January 19, 2022 00:09

Shivam010 requested changes Jan 19, 2022

View reviewed changes

Fix Test skip

64162db

breno12321 requested a review from Shivam010 January 27, 2022 11:33

Shivam010 approved these changes Jan 27, 2022

View reviewed changes

Shivam010 merged commit 16e335f into nitishm:master Jan 27, 2022

Shivam010 mentioned this pull request Jan 27, 2022

JSONQGet + JSON.INDEX ADD #40

Open

Conversation

breno12321 commented Jan 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Uh oh!

Shivam010 left a comment

Choose a reason for hiding this comment

Uh oh!

Shivam010 Jan 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shivam010 Jan 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

breno12321 commented Jan 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Shivam010 commented Jan 18, 2022

Uh oh!

Shivam010 left a comment

Choose a reason for hiding this comment

Uh oh!

breno12321 commented Jan 18, 2022

Uh oh!

Shivam010 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

breno12321 commented Jan 27, 2022

Uh oh!

breno12321 commented Jan 27, 2022

Uh oh!

Shivam010 left a comment

Choose a reason for hiding this comment

Uh oh!

Shivam010 commented Jan 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

breno12321 commented Jan 15, 2022 •

edited

Loading

Shivam010 Jan 15, 2022 •

edited

Loading

Shivam010 Jan 18, 2022 •

edited

Loading

breno12321 commented Jan 15, 2022 •

edited

Loading

Shivam010 left a comment •

edited

Loading