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

Add Net::IMAP::Config class for global and client-scoped config #288

Closed
nevans opened this issue May 24, 2024 · 1 comment · Fixed by #301
Closed

Add Net::IMAP::Config class for global and client-scoped config #288

nevans opened this issue May 24, 2024 · 1 comment · Fixed by #301
Assignees
Milestone

Comments

@nevans
Copy link
Collaborator

nevans commented May 24, 2024

Motivation

The primary motivation is to simplify backward compatibility concerns while still allowing deprecation and breaking changes. Users should be able to opt-in to new (incompatible) behaviors before they become the default, opt-out of new (incompatible) behaviors after they become the default, and load defaults by version number (similarly to how Rails handles upgrades via Rails::Application::Configuration#load_defaults).

The secondary motivation is to scope all config options. It should be possible to globally update defaults for client options and method parameters. But client configuration should be able to override any global configuration. A simple mechanism for customizing configuration inheritance should be available.

Another motivation is to avoid a proliferation of configuration methods on Net::IMAP, both global config attributes (class methods) and client config attributes (instance methods). Currently, the number of config options is low, but that number will go up to handle options for backward and forward compatibility.

An implementation goal, not originally in this proposal but discovered during #291: Basic type checking and coercion should be simple. For example, the timeouts are already being assigned using @foo_timout = Integer(foo_timout), so the config options for them should do that too.

Non-goals

The following were considered, but are not goals for this issue (maybe in future PRs):

  • client-specific configuration (such as host and port).
  • Reading config from ENV, Thread locals, Fiber locals, or Fiber storage.
  • Thread safety (config is usually only changed up-front or during client construction)
  • Ractor safety

Proposal

Net::IMAP::Config will have attribute readers/writers for all config options.

All config objects (except for the default config) will inherit from a #parent config object. All configs will have a hard-coded (frozen) Config.default as their final ancestor, and Config.default will be the only config with parent.nil?. Net::IMAP.config will be a global config (not frozen), which inherits from default.

Every client will create its own unique Config in #initialize, and all unknown keyword args will be forwarded to the client config attributes. A config keyword arg will set the client's parent config. This would allow a client to use defaults without inheriting from the (mutuble) global config, or to inherit from a custom ENV or Fiber.storage based config, which might itself inherit from the global config. With no further customization, the config chain is client -> global -> default.

Version compatibility defaults will be provided as hard-coded frozen configs, which override most values but inherit from global in order inherit options like debug or logger. If a client opts in to using specific version defaults, the config chain will be something like client -> defaults_for(0.4) -> global -> default. Alternatively, these defaults can be copied into an existing config: config.load_defaults(0.4).

The current default config should be stable; it should only change when config options are added or when the minor version number is bumped. Backward compatibility defaults should only change as config options are added. Deprecated config options should only be dropped when the minor version is bumped.

Config Options

Existing global settings

  • debug

Existing client settings

  • open_timeout
  • idle_response_timeout

Existing method parameters

  • #authenticate
    • sasl_ir
    • sasl_registry -> registry kwarg
  • #id
    • client_id
  • #idle
    • idle_timeout -> timeout argument
    • idle_response_handler -> response_handler block argument
  • #select/#examine
    • select_condstore -> condstore kwarg

Ideas for future config options

@nevans
Copy link
Collaborator Author

nevans commented Jun 13, 2024

As I wrote in a comment on #291, this is the last significant feature I want for the 0.4 branch. With this in place, we can choose to backport deprecation warnings, with or without any new feature or behavior. For example, I want the default config in v0.6 to disallow all implicit use of RawData, but that requires big updates to search (because currently complex searches need to be sent as a string). Those big updates will not be backported to 0.4, but I may backport the option to enable the deprecation warnings: it would still be useful to get warnings (and maybe convert that code to explicitly use RawData).

@nevans nevans linked a pull request Jun 22, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant