Go-github: Allow setting of "Accept" header outside methods

[skakim]

Me and @gabrielseibel1 were trying to use go-github (awesome repository, btw!) in a project to trigger an webhook in Github.

We were using the Dispatch method created on #1306 by @fleskesvor, but we were receiving the following message as response:

POST <url>/dispatches: 415 If you would like to help us test the Dispatching custom events for a repository through webhooks during its preview period, you must specify a custom media type in the 'Accept' header. Please see the docs for full details. []

After some research, we discovered that the Github API of our Github Enterprise is older than github.com, it still requires the req.Header.Set("Accept", mediaTypeRepositoryDispatchPreview) to be set, and it was removed in #1391.

Going back to v29.0.2 solved our issue, but we decided to raise an Issue to suggest the following (as we also would like to keep using the latest version whenever possible): Instead of manually adding the "Accept" header inside the methods, could we instead define it via a method parameter or create a class attribute that can be set by the user of the module?

This would increase the retro-compatibility, that is needed when someone is using Github Enterprise in older versions. The only pain would be to have the specific headers for each API call already prepared for the different Github versions, but maybe is just a matter of reverting the commits that remove them from the github/github.go file.

[gmlewis]

Hmmm... That's an interesting idea... My initial gut reaction is "isn't this going to make maintenance of this repo even more time consuming?"

• When do custom headers ever get removed? Must we keep them around for perpetuity?
• How do we make it so that Enterprise users can easily find the version and headers they need?
• How do we keep this from affecting the large number of users that are hitting the public GitHub server?

I probably have more questions, but I'm typing on an Android keyboard so I'll stop for now. 😁

[skakim]

Hmmm... That's an interesting idea... My initial gut reaction is "isn't this going to make maintenance of this repo even more time consuming?"

I agree, and part of this I think is due to Github's API strategy that is strange to me. Nevertheless, as it is implemented this way, it would be good if this library would be easy to use even with this strategy.

• When do custom headers ever get removed? Must we keep them around for perpetuity?

I think yes, due to backward compatibility. We may create a file with all these headers, and in the methods we can first try to do the API call without them, and if them fail, we use the header.

I don't know if there are different headers possible for a method. If that is true, this can be harder to implement, as we will need to know which to use. This connects with the next question.

• How do we make it so that Enterprise users can easily find the version and headers they need?

I wonder if we can recover from an API the Github Version and, using it, we can automatically decide which headers to use and which to not use. Github should have a table or something similar stating for each version (Everest, Comfort-fade, etc) which headers have been added/removed. If we cannot, then we would need to do it manually, and I agree this would be a pain to do and to maintain.

Maybe we can see how other repositories, like PyGithub/PyGithub, deal with this problem, if they deal with it?

• How do we keep this from affecting the large number of users that are hitting the public GitHub server?

This would need to be somewhat automatic or at least something that the user in Github Enterprise needs to define manually. By default the library would be synced with the public GitHub, to not affect the majority of the users.

I probably have more questions, but I'm typing on an Android keyboard so I'll stop for now. 😁

No problem, thank you for taking your time to see this Issue and think through it! 😄

[gmlewis]

Maybe we can see how other repositories, like PyGithub/PyGithub, deal with this problem, if they deal with it?

That sounds like a great idea. Also, over the years, the wonderful people at [email protected] have consistently been super friendly, patient, and helpful with me and my questions, and I know they have for others too.

We've also had some GitHub people contribute to this repo, which is awesome!

So maybe they will take a look at this and give their opinions, or you could reach out to them and see if they have recommendations.

I'm not adverse to this idea... but I also realize that I will not be maintaining the repo forever and don't want to place unnecessary burden upon future maintainers if we can somehow come up with an elegant and low-maintenance solution. :smile:

[gmlewis]

Oh, and I forgot to address one of your concerns:

I don't know if there are different headers possible for a method. If that is true, this can be harder to implement, as we will need to know which to use.

Yes, absolutely multiple custom headers are sometimes accepted and required. If you search for strings.Join in this repo, you will undoubtedly find some. :smile:
Hah... I just did this myself, and the vast majority of cases where we use strings.Join is exactly for multiple custom headers. :smile:

[JoelPagliuca]

My workaround is building the request myself, then json decoding the results into a CodeSearchResult struct defined by this package (which is extremely useful btw! :heart:).

This isn't too onerous, but obviously I'd prefer something less verbose, such as attaching a value that I've copied from the Github docs (in this example application/vnd.github.v3.text-match+json) onto the Client perhaps?

I don't feel like as a user of this package I'd be trying to discover Accept headers from anywhere outside the Github docs themselves. I tend to just try API requests, realize I'm missing some data from my responses, then scour the Github docs for a missing Accept header.