Enable caching of GitHub API calls

[kohsuke]

Use OkHttp with caching so that repeated access to GitHub API won't hit the rate limit too easily.

@reviewbybees

[ghost]

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

[jenkinsadmin]

Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests.

[jglick]

This looks very generic and should be in the github-api library (or plugin), not here. At most this plugin should need to make a one-line .withCaching() call to enable this optimization.

[KostyaSha]

If you need username/password then you are on the wrong way. Please don't expect github-plugin support for not recommended and insecure usage.

[jglick]

We do not ever recommend use of passwords, and it is quite appropriate to make help text guide users toward access tokens (and even implement form validation to warn about things that do not look like access tokens), but I see no reason to arbitrarily block a user from using something that in fact works.

[KostyaSha]

Because user/password provides full access and no control. GH (and other applications) used it on first steps, nowadays it should never be used.

[lanwen]

Github-plugin provides ability to convert login+pwd to token easily. So it any other can point to this ability if anybody wants to use login and password. There is also static method to do that, so it can be used transparently

[KostyaSha]

Oh, right. Afair the idea was to have helper UI elements, but checkboxes looks ugly, so @lanwen placed this under advanced button (that is not collapsible (pure jelly form) and also looks not good) :(

[KostyaSha]

If you continue duplicating features, please ensure that you wouldn't use cache directory that used by github-plugin.

[kohsuke]

github-api library can't really do much beyond what it already does. OkHttp isn't a hard dependency for it, and the code that activates the caching is already a one liner. Given the use of OkHttp in the github plugin, I've moved OkHttp up into the github-api plugin, but all it does is to remove one copy/paste class.

As you see in the comment I left, without further cooperation from the github plugin, we can't really do more code reuse. FWIW, I'm with Jesse that we shouldn't be in the business of blocking people from doing things that work. Creating a personal access token and figuring out the right scopes to give is non-trivial.

@KostyaSha, you want other people to reuse the github plugin more, and so do I. Yet in another comment in this PR, you seem to be just completely unwilling to work with us. I'm confused.

[KostyaSha]

I've moved OkHttp up into the github-api plugin, but all it does is to remove one copy/paste class.

You did unreviewed changes and release in master. Have you tested some jenkins plugins before making API release?

Creating a personal access token and figuring out the right scopes to give is non-trivial.

Hope that a joke. Hope you are not using root users on your desktops even if it technically allowed.

Yet in another comment in this PR, you seem to be just completely unwilling to work with us. I'm confused.

Cache directory from github plugin code should be used only for github-plugin either it may get wipe or any other modifications for unknown to plugin servers.

I'm replying on my holiday in time to your changes. Of course this plugin not under my control, so feel free to fill issue/PR and provide any intelligible explanations why people should do bad things.

@lanwen FYI

[lanwen]

what for this method?

[kohsuke]

The method creates a GitHub object from the connection parameters, typically a user name and a personal access token.

[lanwen]

so maybe just return GitHubPlugin.configuration().findGithubConfig(and(withHost(apiUrl), server -> StringUtils.equals(server.getCredentialsId(), credentials.getId())) ?
(with replacement of lambda to guava Predicate)

[kohsuke]

You did unreviewed changes and release in master. Have you tested some jenkins plugins before making API release?

Yes, no worries, though there's always more testing one can do. I hope it helps github-plugin too by avoiding its own copy of code copied from github-api.

I'm replying on my holiday in time to your changes. Of course this plugin not under my control, so feel free to fill issue/PR and provide any intelligible explanations why people should do bad things.

I'm sorry to file a PR on some random plugin on your holiday. Don't let me keep you.