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

Upgrade to v3.14.9 and remove okhttp3 url-connection #3

Merged
merged 8 commits into from
Jun 18, 2020

Conversation

bitwiseman
Copy link
Contributor

@bitwiseman bitwiseman commented May 29, 2020

I've published an initial v3.12.12 with the url-connection still included.

This updates to the latest v3.14, which does not have url-connection any more.

Note: okhttp 2.7.5 urlconnection is retained as that is much more widely used.

@bitwiseman
Copy link
Contributor Author

Also, we discussed pulling ObsoleteUrlFactory.java into this plugin. That would make it the responsibility of this plugin to maintain that obsolete API for use.

@dwnusbaum
Copy link
Member

It's hard to evaluate this without seeing PRs to other plugins currently using OkHttp to understand whether this would affect them being able to use the API plugin. Can we get draft PRs filed for downstream plugins to understand the impact?

The urlconnection has not been support for many years and is incompatible with
a number of okhttp3 features.  Better to not include it.

See https://square.github.io/okhttp/changelog_3x/#version-3140 and copy down the
ObsoleteUrlFactory if you absolutely must, but we do not want to have this plugin forcing that dependency.
@bitwiseman
Copy link
Contributor Author

bitwiseman commented Jun 2, 2020

The move to a version of okhttp3 that does not support HttpUrlConnection should only effect plugins that depend that feature (or have transitive dependencies on it). Kubernetes client doesn't depend on it:

https://github.com/fabric8io/kubernetes-client/blob/2ed58668ab6a0f83e038844d96efbbf4eea92a9c/kubernetes-client/pom.xml#L126-L134

The version change may still have some effect. I'll need to create script of some sort to go looking for okhttp3 in the transitive dependencies of jenkins plugins.

@bitwiseman
Copy link
Contributor Author

bitwiseman commented Jun 16, 2020

@dwnusbaum

Summary

To summarize our offline discussion and my analysis:

The github-api-plugin is the only current consumer of this plugin. Thus the main concern in updating this plugin to a version that does not support okhttp3.okhttp-urlconnection would be the following scenario:

  • another plugin depends on github-api-plugin AND
  • that plugin (or some plugin that depends on it) also requires okhttp3.okhttp-urlconnection

I've done analysis on the dependencies and this appears safe.

Details

Only the following plugins depend on github-api-plugin:

DotCi
blueocean-github-pipeline
elasticbox
ghprb
github
github-branch-source
github-coverage-reporter
github-issues
github-oauth
github-pr-coverage-status
github-pullrequest
ignore-committer-strategy
multibranch-build-strategy-extension
multibranch-scan-webhook-trigger
pipeline-githubnotify-step

okhttp3.okhttp-urlconnection

These are the only plugins that bundle okhttp3.okhttp-urlconnection and their dependencies:

azure-acs
+-- azure-commons
+-- azure-credentials
+-- kubernetes-cd
    +-- azure-commons
    +-- azure-credentials
    +-- credentials
    +-- credentials-binding
    +-- docker-commons
    +-- ssh-credentials

azure-ad
+-- azure-commons
+-- cloudbees-folder
+-- configuration-as-code
+-- matrix-auth

azure-app-service
+-- azure-commons
+-- azure-credentials
+-- docker-commons
+-- docker-java-api
+-- git-client

azure-batch-parallel

azure-commons
+-- jackson2-api
+-- ssh-credentials
+-- worflow-step-api

azure-container-agents
+-- azure-commons
+-- azure-credentials
+-- docker-commons
+-- plain-credentials
+-- ssh-credentials
+-- windows-azure-storage

azure-container-registry-tasks
+-- azure-commons
+-- azure-credentials
+-- scm-api
+-- structs

azure-credentials
+-- credentials
+-- credentials-binding
+-- jackson2-api
+-- plain-credentials
+-- ssh-credentials
+-- structs

azure-function
+-- azure-app-service
+-- azure-commons
+-- azure-credentials
+-- git-client

azure-iot-edge
+-- azure-commons
+-- azure-credentials
+-- docker-commons

azure-vm-agents
+-- azure-commons
+-- azure-credentials
+-- azure-commons
+-- cloud-stats
+---credentials
+---plain-credentials

azure-vmss
+---azure-commons
+---azure-credentials

service-fabric
+---azure-commons
+---azure-credentials
+---structs
+---workflow-durable-task-step

upload-pgyer
+-- structs

None of the plugins that currently bundle okhttp3.okhttp-urlconnection depend (directly or transitively) on github-api-plugin. This change is safe.

@bitwiseman bitwiseman closed this Jun 18, 2020
@bitwiseman bitwiseman reopened this Jun 18, 2020
Copy link
Member

@dwnusbaum dwnusbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes and doc updates look good to me. It would be great to see a linked PR to github-api to test the change.

README.adoc Outdated Show resolved Hide resolved
README.adoc Outdated Show resolved Hide resolved
README.adoc Outdated Show resolved Hide resolved
@bitwiseman
Copy link
Contributor Author

Linked PR showing this working - jenkinsci/github-api-plugin#57

README.adoc Outdated Show resolved Hide resolved
README.adoc Outdated Show resolved Hide resolved
README.adoc Outdated Show resolved Hide resolved
README.adoc Outdated Show resolved Hide resolved
README.adoc Outdated Show resolved Hide resolved
@bitwiseman bitwiseman merged commit c4a6b5b into jenkinsci:master Jun 18, 2020
@bitwiseman bitwiseman deleted the version-3.14 branch June 18, 2020 20:24
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.

2 participants