Skip to content
This repository has been archived by the owner on Apr 8, 2019. It is now read-only.

Client, Request, Response #2

Open
2 tasks
at15 opened this issue Aug 15, 2017 · 3 comments
Open
2 tasks

Client, Request, Response #2

at15 opened this issue Aug 15, 2017 · 3 comments
Assignees
Milestone

Comments

@at15
Copy link
Owner

at15 commented Aug 15, 2017

We need to design a standard struct/interface for client, request, response to use across all the operations, schema changes, updating documents etc. It is possible we do the converting directly and user don't need to call json.Unmarshal directly

TODO

  • checkResponse does not work for certain APIs because
    • they don't use status code to indicate error
    • 404 returns html page
    • schema management has its own error response format

Ref

@at15 at15 added this to the 0.1.0 milestone Aug 15, 2017
@at15 at15 self-assigned this Aug 15, 2017
@at15
Copy link
Owner Author

at15 commented Aug 16, 2017

there is one problem of how to organize the files, we can put all the files under one package

schema_core.go
schema_filed.go
admin_info.go
client.go

or use multiple packages

schema
   - core.go
   - field.go
admin
   - info.go
client.go

The main problem of using multiple packages is there is import cycle for client, one way to solve it to have a service wrapper different packages, so the user is calling the service instead of the client package directly

at15 added a commit that referenced this issue Aug 16, 2017
- mainly modeled after go-github #2
- the reuse service one is not working, better write different service
struct for different services
@dio
Copy link

dio commented Jan 29, 2018

I'm interested in google/go-github#317 as one of the listed items. As the merged patch is: https://github.com/google/go-github/blob/master/github/github.go#L482-L486 do you think this is still required for a client in newer current golang version?

@at15
Copy link
Owner Author

at15 commented Jan 31, 2018

@dio based on comments in the original issue, it seems net/http has already fixed that in https://go-review.googlesource.com/c/go/+/21291.

There are some similar issues go-kit/kit#249 which leads to golang/go#15703 then leads to https://go-review.googlesource.com/c/go/+/23200

This verifies that the client's bufio reading reads ahead and notes the EOF, so even if the JSON
decoder doesn't read the EOF itself, as long as somebody sees it, a close won't forcible tear down the connection. This was true at least of https://golang.org/cl/21291

so I believe it is not needed in latest version, but I haven't run any experiment.

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

No branches or pull requests

2 participants