Conversation
| /// Factory function type for creating schedulers from configuration | ||
| pub type SchedulerFactory = Arc<dyn Fn(&config::Config) -> Box<dyn Scheduler + Send> + Send + Sync>; | ||
|
|
||
| static REGISTRY: OnceLock<Mutex<HashMap<String, SchedulerFactory>>> = OnceLock::new(); |
There was a problem hiding this comment.
This is a request for a comment on the registry
| } else { | ||
| Self::new(depth, iterations) | ||
| } | ||
| } |
There was a problem hiding this comment.
I feel like these shouldn't exist and that constructing a scheduler from a config should be done via the regular scheduler creation apis and live with the config
| let mut map = HashMap::new(); | ||
|
|
||
| // Register built-in schedulers | ||
| map.insert( |
There was a problem hiding this comment.
Why do we insert instead of calling register_scheduler?
| File(Option<std::path::PathBuf>), | ||
| } | ||
|
|
||
| impl FailurePersistence { |
There was a problem hiding this comment.
These things might be easier with https://docs.rs/strum/latest/strum/
|
|
||
| fn should_enable_metrics(global_config: &config::Config) -> bool { | ||
| global_config.get_bool("enable_metrics").unwrap() | ||
| } |
There was a problem hiding this comment.
These things should be impled on &self where self is the config and live somewhere else
|
Thanks! I hate to be a hater, but I'm generally not a fan of how this has been structured. I'd like for the config stuff to be as far as possible "its own thing", meaning it really should be just changes in one file. |
|
I think this is the least important PR to get merged this week. If you don't get to it, then I'll take on the work to drive it to merged. |
|
Cancelling in favor of reopening or creating a new PR in the future. I think this should be structured a bit differently. |
2 participants
This PR adds a global config for Shuttle, utilizing the config crate to make Shuttle tests configurable via a TOML file and/or environment variables. This allows users to swap scheduling algorithms, configuration parameters, or (in the future #217) time models declaratively without editing or recompiling their test code.
The public
checkfunction now uses this global config when running Shuttle tests. All other entrypoints to Shuttle (check_random, building aRunnermanually, etc.) are unchanged.Currently, the global config is the same for all tests using
check, but it is easy to extend this in a future PR to store "sub-configs" in the global config objects, which can be accessed in subsets of tests via a keycheck(fn, key)API.To run a scheduler via the global config, that scheduler must register a "factory" function that can create instances of that scheduler with the scheduler "registry". The scheduler "registry" is a global static that maps the name of the scheduler used in the config (
scheduler.type) to the factory function. All basic schedulers in the Shuttle repository are registered by default, but additional custom schedulers can also be registered dynamically so that they can be run via the global config / check function.Documentation
shuttle/shuttle_config_example.toml-- provides an example config with most configuration options for the included Shuttle schedulersfn check-- the doc comment explains the configuration precedence and environment variable naming conventionBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.