Added basic http client #7
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
@tobiemh I think this might be a bit confusing, as it conflicts with the standard golang http package, and having a |
Ahhh yes, of course - good point! |
|
A general comment if I may. Two things concern me, the language of xxxAll and the use of strings. My mental model is something along the lines of (using create for an example) Create() and CreateOne(). My second concern is the use of string as the value param. I really would love to see this as interface{} that is marshaled inside Request so we can pass structs vs marshaling before calling. |
|
@intabulas The first one is sorta outta my control, I was just using the functions from https://github.com/surrealdb/surrealdb.py/pull/5/files which has been merged already, as well as when going to the official docs for things like https://surrealdb.com/docs/integration/http#select-all it'll bring you to the query... Though I do admit its a bit strange. I agree on the second point! String was just the easiest, and now that the PRs for marshalling are in, ill revisit this and see what I can do |
|
@MrShutCo gotcha, totally makes sense to follow the pattern then |
|
@intabulas @tobiemh The PR is ready for a proper review. I've put a bunch of TODO's as well for other devs to possibly improve on this code. It isn't perfect, but hopefully its enough of a base for people to improve on :) |
|
Hi @MrShutCo just looking at this again, and thinking about the upcoming Rust library, what is the need to have an HTTP/REST specific functionality available to the end user? With regards to the Rust library (and the latest changes to this Golang library), the HTTP/WebSocket implementations are hidden from the user. What are your thoughts/reasoning? |
|
@tobiemh I was just following what was being done on other repos like the Python one. If we're not exposing the endpoints and Websockets directly then this PR has no real use, and I must have missed some of the discussion on how it would be used |
|
Hey @MrShutCo ah no don't worry you haven't missed the discussion. I'm going to keep this PR open for the moment, but if we follow the Rust library (coming soon), then the HTTP endpoints will be used behind the scenes for |
|
Heya! Checking out open changes - I think there is still some work to be done in this area, so can't quite approve yet, but we will get back to it. Thanks for the work and patience! |
|
Hi @MrShutCo ! Would you be able to merge the latest changes so we can get this in? It would be really cool to have in included! |
|
Landing this, thank you for persistence @MrShutCo ! |
|
|
||
| resp, err := client.CreateOne("personreplace", "surreal", `{child: null, name: "FooBar"}`) | ||
| IsEqual(t, nil, err) | ||
| fmt.Println(resp.Result) |
There was a problem hiding this comment.
Could we change these println to testify asserts please?
|
Waiting for asserts to replace println then will approve @MrShutCo |
|
@phughk What should be done about the response object? Ive tried to replace the fmt statements but its kinda hard since the objects look like I also tried SmartUnmarshal but its built just for websockets stuff and fails on my end |
|
We can use the complex types for now and address in the future but agree |
|
Hey @MrShutCo , could you please add in a new PR to skip all the new code updates? Or else merge this PR? |
Added a basic wrapper struct that allows http requests to the POST /sql endpoint, returning the response a string of the json. Also added the testify library for writing unit tests