-
Notifications
You must be signed in to change notification settings - Fork 9
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
WIP: Example plugin in Rust #11
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 🚀
Some comments, mostly about simplification.
examples/fitbit-rs/src/utils.rs
Outdated
if let Some(host_cookies) = cookies.get(hostname) { | ||
Ok(host_cookies.clone()) | ||
} else { | ||
Err(format!("Cannot find cookies for {}", hostname)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we can do
cookies
.get(hostname)
.cloned()
.ok_or_else(|| format!("Cannot find cookies for {}", hostname))
examples/fitbit-rs/src/utils.rs
Outdated
use extism_pdk::*; | ||
|
||
pub fn get_cookies_by_host(hostname: &str) -> Result<HashMap<String, String>, String> { | ||
let cookies_result = get("cookies"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the function would return anyhow::Error
instead of String
like extism-pdk does, we can make use of ?
let cookies = get("cookies")?;
But even without it we can do
let cookies = get("cookies").map_err(|err| err.to_string())?;
examples/fitbit-rs/src/utils.rs
Outdated
let cookies_json = match cookies_result { | ||
Ok(Some(json_str)) => json_str, | ||
Ok(None) => return Err("No cookies found in the configuration.".to_string()), | ||
Err(e) => return Err(format!("Error retrieving cookies: {}", e)), | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then this becomes
let cookies = cookies.ok_or_else(|| "No cookies found in the configuration.".to_string())?;
} | ||
|
||
#[plugin_fn] | ||
pub fn start() -> FnResult<Json<bool>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of a bool
it would be nicer to have a anyhow::Result<()>
. Then you can also add error messages similarly to how you do in utils.rs
.
} | ||
|
||
#[plugin_fn] | ||
pub fn two() -> FnResult<Json<Option<String>>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function never returns an error, but logs it. Is this because of the plugin_fn
constraints?
This plugin does not work yet. It needs to access local storage to get the currently hardcoded user id |
No description provided.