Replace direct usage of http.DefaultClient in providers and oauth2 calls #87
Conversation
felixLam
force-pushed
the
feature/httpClient
branch
from
2ddd192
to
e3a162c
Compare
felixLam
force-pushed
the
feature/httpClient
branch
from
176de61
to
8c5bca6
Compare
felixLam
changed the title
|
Why not have the |
|
I thought so too, but you would still need a field for each provider in order to let you set it externally. The Client field was also already available in some providers. The current implementation requires less code duplication unless I am missing something. |
|
@markbates Did you have time to look at this PR again? If you have any suggestions I am more than happy to implement them. |
|
I'm sorry, I haven't. I'm traveling through Italy right now with my family, so I don't have the time nor the Internet to really investigate. I plan on checking this out when I get to gopher con the end of next week. |
|
Enjoy your holidays 🇮🇹 and make sure to watch the match with the locals tomorrow ⚽ |
|
Hi @markbates any news on this PR? |
|
@markbates any news on this PR? Is it worthwhile updating it to resolve conflicts? |
|
@markbates This would really help us out in Gitea to set custom fields in the Client! |
|
@willemvd it is implemented on our branch in this PR. If you care and plan on using it I can rebase it on the current master. Since @markbates has gone radio silent here, I haven't looked into it. |
|
@felixLam since @markbates is actually doing stuff on the master (last time 2 days ago) I hope he will respond on this, but a rebase is always welcome! |
…lementing SkipVerify (waiting for markbates/goth#87)
# Conflicts: # providers/amazon/amazon.go # providers/cloudfoundry/session.go # providers/github/github.go # providers/gitlab/gitlab.go # providers/influxcloud/influxcloud.go # providers/influxcloud/session.go # providers/instagram/instagram.go
felixLam
force-pushed
the
feature/httpClient
branch
from
f534bab
to
36cd39a
Compare
|
@willemvd I merged master into this PR (didn't do a rebase as we have our own branch on top of this PR). Let me know if this works. |
|
@felixLam found out that the new introduced auth0 provider is not updated with the new Provider.Client field |
|
Weird that this is not raised by the CI. I ran the test locally and they passed, can you let me know what command you used that failed? |
|
Simply go build ? |
|
I just looked at the code and saw that it was missing (think also in the session.go) 😄 |
|
@markbates Thanks! |
Some providers already used a custom httpClient. This brings the
Clientfield to all providers and uses a fallback method to provide thehttp.DefaultClientif no client is set.This also uses the custom httpClient for the oauth2 calls via the context.
The change should allow better inversion of control during unit tests by providing a custom client.
I also ran go format on some files to clean up the import statements.