feat: Support waiting for different client states #111
Conversation
Signed-off-by: Travis <[email protected]>
rainerschoe
force-pushed
the
wait_for_init_config
branch
from
2d2c8c5 to
0557b0c
Compare
| let (current_mode_mutex, condition_variable) = &*self.current_mode; | ||
| let mut current_mode = current_mode_mutex.lock().unwrap(); | ||
| *current_mode = CurrentMode::Defunct(result.clone()); | ||
| condition_variable.notify_all(); |
There was a problem hiding this comment.
This is kind of a repeating pattern when working with condition variables.
WDYT should we add an abstraction to not always deal with the tuple and weird derefs? Something like:
use std::sync::{Mutex, Condvar};
struct Waitable<T> {
value: Mutex<Option<T>>,
condvar: Condvar,
}
impl<T> Waitable<T> {
fn new() -> Self {
Self {
value: Mutex::new(None),
condvar: Condvar::new(),
}
}
fn set(&self, val: T) {
let mut guard = self.value.lock().unwrap();
*guard = Some(val);
self.condvar.notify_all();
}
fn wait(&self) -> T {
let mut guard = self.value.lock().unwrap();
while guard.is_none() {
guard = self.condvar.wait(guard).unwrap();
}
guard.take().unwrap()
}
}
There was a problem hiding this comment.
yes, probably. There are some lines that are repeated again and again.
Signed-off-by: Travis <[email protected]>
rainerschoe
force-pushed
the
wait_for_init_config
branch
from
8172e03 to
c8d06c9
Compare
Signed-off-by: Travis <[email protected]>
| /// For remote configurations, it returns whether it's connected to the | ||
| /// remote or not | ||
| fn is_online(&self) -> Result<bool>; | ||
|
|
||
| /// For remote configurations: Blocks until it's connected to the remote. | ||
| fn wait_until_online(&self); |
There was a problem hiding this comment.
We didn't want a different trait when we had only is_online, maybe now we can think about adding it
| /// Blocks until a configuration is available. | ||
| /// Note: This is different than wait_until_online, as configuration could be available | ||
| /// through alternate sources (Cache / Fallback) | ||
| fn wait_until_configuration_is_available(&self); |
There was a problem hiding this comment.
Not sure if we need this as a first-class citizen here. Couldn't this information be retrieved via the initial config + is_online (+ maybe we need a way to get current status)?
| fn wait_until_online(&self) { | ||
| error!("Waiting for AppConfigurationOffline to get online. This will never happen."); | ||
| std::thread::park(); // block forever | ||
| } |
There was a problem hiding this comment.
Note my previous comment about using a different trait. This and Configuration shouldn't implement these methods.
| Ok(self.get_current_mode()? == CurrentMode::Online) | ||
| } | ||
|
|
||
| fn wait_until_configuration_is_available(&self) { |
There was a problem hiding this comment.
... I'm not really sure if we want to implement this behaviour. What is the use-case for this method?
| let (current_mode_mutex, condition_variable) = &*self.current_mode; | ||
| let mut current_mode = current_mode_mutex.lock().unwrap(); | ||
| *current_mode = CurrentMode::Defunct(result.clone()); | ||
| condition_variable.notify_all(); |
There was a problem hiding this comment.
yes, probably. There are some lines that are repeated again and again.
closes #67
This implements waiting functions to allow the user to wait for the client to reach a certain state.
Thinking about #107 I think it is a good idea to distinguish waiting for a configuration to be available and waiting for getting online.
We can already serve features and properties even though we did not yet establish a connection with the server.