Go client driver #465
Go client driver #465
Conversation
These row iterators conform to the database/sql/driver interface requirements. Since these functions perform useful data conversions, I've implemented these at this level so that they can also be used by someone who does not want to go through database/sql.
Rollback should not return an error if we're not in a transaction. Many frameworks generally perform a rollback for good measure. So, returning an error just causes confusion.
|
Your commit doesn't complain about missing comments on public methods? How do you do that? :) LGTM in general, I'd have a few nitpicks here and there but nothing much. |
|
The tools make an exception when they see my name :). |
|
Ah I see now. That makes sense. I had seen it somewhere else and I was wondering. I'll use the non-exported type trick more often then. :) |
| ) | ||
|
|
||
| // fakeVTGateService has the server side of this fake | ||
| type fakeVTGateService struct { |
There was a problem hiding this comment.
We already have a fakeVTGateService, what's the point to not reuse it ? https://github.com/youtube/vitess/blob/master/go/vt/vtgate/vtgateconntest/client.go
In the meanwhile, I'd like to bring up a question that maybe we could consolidate our fake structs so that people could easily reuse existing ones.
There was a problem hiding this comment.
We should talk about it. As I mentioned in the other issue, there are pros & cons.
There was a problem hiding this comment.
The wrangler/testlib code is a good example of a library meant to be used by tests. I wasn't arguing against that, I just didn't want the fake to depend on bson RPC earlier. Ideally, fakes and test utility functions should be in their own libraries.
I would also caution against using too many layers in unit tests: if you're testing the client high level library, connecting through the low level library to the server, you can setup a fake server and connect to it, or you could use a fake client connector, and that would be simpler / easier, and make the unit test more 'unit'...
This is the go client driver. The comments in client.go document how it should be used.