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

spike: deprecated settings all working #43

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

yaauie
Copy link
Contributor

@yaauie yaauie commented Aug 22, 2023

Reference implementation of things I have talked about in #42, with all existing specs passing.

Notably:

After upgrading this mixin dependency, without any code changes to the plugins that include this mixin:

  • at runtime: any in-the-wild user pipeline configuration that references a plugin including this mixin will behave identically.
  • at pipeline start: any plugin that relies on this plugin will allow a user to specify standardized SSL options
  • at pipeline start: any plugin configured with a deprecated setting will log a warning and emit guidance to use the standardized option

This is only a spike, and it largely uses the config code as written in #42 but makes it available to plugins in a manner similar to the one described in #42 (review). It does not port the tests from #42, because I wanted to show all specs passing without modification.

  1. Bulk of functionality is in a new Implementation module
  2. A DeprecatedSslConfigSupport holds all of the deprecated SSL logic
  3. A private Adapter dynamic module can be included into a plugin class with or without the Deprecated SSL settings and exposed through HttpClient#[with_deprecated:]
  4. The HttpClient::included now auto-mixes in Adapter.new(with_deprecated: true) so that no changes are required in codebases that use this plugin

@yaauie yaauie mentioned this pull request Aug 22, 2023
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.

1 participant