-
Notifications
You must be signed in to change notification settings - Fork 123
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 synchronous API wrapper #43
Comments
I'm new to async and not really that new to rust but still a beginner. How would I go about implementing this? Or what should I understand first? |
I think what we'd want here (if we even want this any more with async/await making async code so much easier to write) is a type that wraps an async runtime (tokio probably) + a fantoccini client, and then has synchronous versions of every method on the fantoccini client that just immediately await the future returned by the async version. |
Is it something like this: use tokio::runtime::current_thread::Runtime
use fantoccini;
struct Client {
rt: Runtime,
client: fantoccini::Client,
}
impl Client {
pub fn new(webdriver: &str) -> Client {
Client {
rt: Runtime::new().unwrap(),
client: fantoccini::Client::new("webdriver path"),
}
}
pub fn find(&mut self, search: Locator) -> Element {
let task = async {
client.find(search).await
}
self.rt.block_on(task)
}
} I didn't add any error handling but is the idea right? |
Yup, that seems basically right! |
Do you prefer it as one commit or broken up into multiple commits? |
Multiple commits is best, but it's fine for you to just push as you go -- we can always edit after. Multiple commits is just far easier to review, because I can easily review only what has changed since last time. |
As observed here, there are some pretty good reasons to make the interface to interact with browsers synchronous. Or at least give the option of having the nicer synchronous interface when you don't need the async. We should be able to provide synchronous wrapper types for most operations pretty easily, so that would be a good addition!
The text was updated successfully, but these errors were encountered: