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

Add a clientResponse -> ClientResponse visible package type to make a… #9

Closed
wants to merge 1 commit into from
Closed

Add a clientResponse -> ClientResponse visible package type to make a… #9

wants to merge 1 commit into from

Conversation

gavbaa
Copy link

@gavbaa gavbaa commented Jul 20, 2016

…dding new external clients easier.

@coveralls
Copy link

coveralls commented Jul 20, 2016

Coverage Status

Coverage decreased (-4.07%) to 90.323% when pulling 77c664d on gavbaa:public-clientresponse into bccf0e6 on powerman:master.

@gavbaa
Copy link
Author

gavbaa commented Jul 20, 2016

I was attempting to add my own custom HTTP Client for the JSON-RPC2 bit, because I wanted to do some additional customizations to the request. Exposing ClientResponse allows me to create my client externally without modifying your package.

@powerman
Copy link
Owner

Can you please be more specific, what exactly you need to customize?

I don't think we should make clientResponse exported. All it does is strict validation of received data, which is usually redundant and slowdown things (I have plans to make this strict validation optional in the future, so this type may change in incompatible way to support this option), so you can just create your own struct with 4 fields and unmarshal there.

Probably a better way to customize http request is to provide a way to affect code at https://github.com/powerman/rpc-codec/blob/master/jsonrpc2/http.go#L101 - and this may be a valuable feature.

@gavbaa
Copy link
Author

gavbaa commented Jul 20, 2016

That's a reasonable response. I needed to be able to manage headers for things like custom authorization, so having control of the Client there would also allow for that.

@powerman
Copy link
Owner

I've added support for custom HTTP client: https://godoc.org/github.com/powerman/rpc-codec/jsonrpc2#NewCustomHTTPClient - see usage example at https://godoc.org/github.com/powerman/rpc-codec/jsonrpc2#example-package (clientCustomHTTP).

@gavbaa
Copy link
Author

gavbaa commented Jul 20, 2016

This approach worked perfectly for our needs, thank you.

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.

3 participants