-
Notifications
You must be signed in to change notification settings - Fork 13
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
Download logs #222
Download logs #222
Conversation
1ef3420
to
3c29a2a
Compare
/// Messages returned directly by the TryCp server, rather than relayed from Holochain | ||
#[derive(Debug, serde::Deserialize, serde::Serialize)] | ||
#[serde(rename_all = "snake_case", tag = "type", content = "data")] | ||
pub enum TryCpServerResponse { |
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.
Some type f*ry required to let the JS client decode these types. It's the first time we've returned anything more complicated than a string and the JS client has a special case for strings.
std::fs::read(&conductor_stdout).context(HolochainStdout { id: id.clone() })?; | ||
|
||
let conductor_stderr = player_dir.join(CONDUCTOR_STDERR_LOG_FILENAME); | ||
let conductor_stderr = std::fs::read(&conductor_stderr).context(HolochainStderr { id })?; |
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.
are we cool with these blocking whatever thread happens to be running?
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.
The entire function is being run on a blocking thread -> https://github.com/holochain/tryorama/pull/222/files#diff-db8eb8bbb5e02b33e796b281bce321e0d781f68204bac8f217e5eddbe9e34319R256
This seems to be the established pattern for the trycp_server, to offload the entire function if it's not async
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.
lgtm
No description provided.