Get rid of bootstrap_option_values in Options instance. #21772
Conversation
| @@ -438,17 +438,16 @@ def from_options( | |||
| prior_result: AuthPluginResult | None = None, | |||
| remote_auth_plugin_func: Callable | None = None, | |||
| ) -> tuple[DynamicRemoteOptions, AuthPluginResult | None]: | |||
| bootstrap_options = full_options.bootstrap_option_values() | |||
| assert bootstrap_options is not None | |||
There was a problem hiding this comment.
No need to assert this now, as we know for_global_scope() always returns an object.
| execution = cast(bool, bootstrap_options.remote_execution) | ||
| cache_read = cast(bool, bootstrap_options.remote_cache_read) | ||
| cache_write = cast(bool, bootstrap_options.remote_cache_write) | ||
| global_options = full_options.for_global_scope() |
There was a problem hiding this comment.
The use of bootstrap options was always gratuitous here. We have a fully formed Options instance, so these are just regular global options now.
|
Continuing to chip away at the complexities of OptionsBootstrapper, so we can delegate most of it to Rust... |
It shouldn't be. This is all in the options bootstrapping sequence, which we have never considered part of the API. You're supposed to request subsystem instances from the engine, not manipulate a raw Options instance. That said, good idea to call this out in the release notes, just in case. |
If you have an Options instance you can get the individual
values from the global scope, since every bootstrap
option is a regular global scope option
(the "bootstrap" designation just means "needed for
creating a Scheduler").
Or, if you need all the bootstrap options in a single
OptionValueContainer (say, to pass them en masse
to Scheduler initialization) then you can get that
directly from the OptionsBootstrapper.