search

bitsbeats / drone-tree-config

Drone helper for mono repositories.
Apache License 2.0
103 stars 24 forks source link

Reuse the github client delegate #31

Closed mach6 closed 3 years ago

mach6 commented 4 years ago

Changed the github_client.NewGitHubClient implementation such that it re-uses the same github.Client for all requests.

There is no need to recreate a new delegate for every request. The internal go-github library calls NewRequest using the delegate receiver for all calls. Furthermore, the same credentials (token) and sever (endpoint) are used for all github requests throughout this project.

--

Also, in this change set -- Updated github_client_test and main plugin_test code to use a fixed testMux across all tests in the same file. This change ensures "the contract" -- a single server and token will be configured -- is also true for the tests.

Notable limitation: Per this implementation, "the contract" is not enforced or checked within github_client.go .. Calling code such as plugin.go needs to comply by never passing a different server or token -- which now that I've updated the tests, is the case everywhere in this project. I'm happy to add a check to detect a change to the server or token which is passed, if there is a favorable view for it.

foosinn commented 4 years ago

This needs some testing on our side, since we have been compiling drone-tree-config directly into our drone fork.

I hope to get this done during the next weeks.

mach6 commented 4 years ago

Sounds good.

FYI, I do something similar. I compile my fork of drone-tree-config into a private drone config extension. I have had this change running for about a month now on a reasonably large deployment with several allow listed monoliths. Thus far, I have not encountered any issues.

That said, I’m happy to wait until y’all vet it too..

I appreciate you letting me know!