Skip to content

Commit

Permalink
add no-sanbox arg to Kaleido process
Browse files Browse the repository at this point in the history
 - something fishy is happening in the CI, without this argument
   empty files are generated because of chromium security issues
 - print stderr of Kaleido to console

Signed-off-by: Andrei Gherghescu <[email protected]>
  • Loading branch information
andrei-ng committed Dec 20, 2024
1 parent 9989551 commit 4973679
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 22 deletions.
1 change: 0 additions & 1 deletion examples/kaleido/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,3 @@ edition = "2021"

[dependencies]
plotly = { path = "../../plotly", features = ["kaleido", "kaleido_fetch"] }
# plotly = { path = "../../plotly", features = ["kaleido"] }
15 changes: 7 additions & 8 deletions plotly/src/plot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,7 @@ impl PartialEq for Plot {
mod tests {
use std::path::PathBuf;

use base64::{engine::general_purpose, Engine as _};
use serde_json::{json, to_value};

use super::*;
Expand Down Expand Up @@ -748,7 +749,7 @@ mod tests {
assert!(!dst.exists());
}

#[cfg(target_os = "linux")]
#[cfg(not(target_os = "macos"))]
#[test]
#[cfg(feature = "kaleido")]
fn test_save_to_png() {
Expand All @@ -760,7 +761,7 @@ mod tests {
assert!(!dst.exists());
}

#[cfg(target_os = "linux")]
#[cfg(not(target_os = "macos"))]
#[test]
#[cfg(feature = "kaleido")]
fn test_save_to_jpeg() {
Expand All @@ -772,7 +773,7 @@ mod tests {
assert!(!dst.exists());
}

#[cfg(target_os = "linux")]
#[cfg(not(target_os = "macos"))]
#[test]
#[cfg(feature = "kaleido")]
fn test_save_to_svg() {
Expand All @@ -796,7 +797,7 @@ mod tests {
assert!(!dst.exists());
}

#[cfg(target_os = "linux")]
#[cfg(not(target_os = "macos"))]
#[test]
#[cfg(feature = "kaleido")]
fn test_save_to_pdf() {
Expand All @@ -820,12 +821,10 @@ mod tests {
assert!(!dst.exists());
}

#[cfg(target_os = "linux")]
#[test]
#[cfg(not(target_os = "macos"))]
#[cfg(feature = "kaleido")]
fn test_image_to_base64() {
use base64::engine::general_purpose;
use base64::Engine;
let plot = create_test_plot();

let image_base64 = plot.to_base64(ImageFormat::PNG, 200, 150, 1.0);
Expand All @@ -850,8 +849,8 @@ mod tests {
assert!(image_base64.is_empty());
}

#[cfg(target_os = "linux")]
#[test]
#[cfg(not(target_os = "macos"))]
#[cfg(feature = "kaleido")]
fn test_image_to_svg_string() {
let plot = create_test_plot();
Expand Down
3 changes: 2 additions & 1 deletion plotly_kaleido/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ fn main() -> Result<()> {

extract_zip(&dst, &kaleido_zip_file)?;
} else {
println!("'download' feature disabled. Kaleido binary won't be downloaded and must be installed manually.")
let msg = "'download' feature disabled. Please install Kaleido manually and make the environment variable 'KALEIDO_PATH' point to it.".to_string();
println!("cargo::warning={msg}");
}
Ok(())
}
39 changes: 27 additions & 12 deletions plotly_kaleido/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ impl Kaleido {
"--disable-dev-shm-usage",
"--disable-software-rasterizer",
"--single-process",
"--disable-gpu",
"--no-sandbox",
])
.stdin(Stdio::piped())
.stdout(Stdio::piped())
Expand Down Expand Up @@ -225,16 +227,29 @@ impl Kaleido {
let output_lines = BufReader::new(process.stdout.take().unwrap()).lines();
for line in output_lines.map_while(Result::ok) {
let res = KaleidoResult::from(line.as_str());
if let Some(image_data) = res.result {
// TODO: this should be refactored
// The assumption is that KaleidoResult contains a single image.
// We should end the loop on the first valid one.
// If that is not the case, prior implementation would have returned the last
// valid image
return Ok(image_data);
match res.result {
Some(image_data) => {
// TODO: this should be refactored
// The assumption is that KaleidoResult contains a single image.
// We should end the loop on the first valid one.
// If that is not the case, prior implementation would have returned the last
// valid image
return Ok(image_data);
}
None => {
println!("empty line from Kaleido stdout");
}
}
}

// Don't eat up Kaleido/Chromiu erros but show them in the terminal
let stderr = process.stderr.take().unwrap();
let stderr_lines = BufReader::new(stderr).lines();
for line in stderr_lines {
let line = line.unwrap();
eprintln!("{}", line);
}

Ok(String::default())
}
}
Expand Down Expand Up @@ -297,7 +312,7 @@ mod tests {
}

// This seems to fail unpredictably on MacOs.
#[cfg(target_os = "linux")]
#[cfg(not(target_os = "macos"))]
#[test]
fn test_save_png() {
let test_plot = create_test_plot();
Expand All @@ -309,7 +324,7 @@ mod tests {
}

// This seems to fail unpredictably on MacOs.
#[cfg(target_os = "linux")]
#[cfg(not(target_os = "macos"))]
#[test]
fn test_save_jpeg() {
let test_plot = create_test_plot();
Expand All @@ -321,7 +336,7 @@ mod tests {
}

// This seems to fail unpredictably on MacOs.
#[cfg(target_os = "linux")]
#[cfg(not(target_os = "macos"))]
#[test]
fn test_save_webp() {
let test_plot = create_test_plot();
Expand All @@ -333,7 +348,7 @@ mod tests {
}

// This seems to fail unpredictably on MacOs.
#[cfg(target_os = "linux")]
#[cfg(not(target_os = "macos"))]
#[test]
fn test_save_svg() {
let test_plot = create_test_plot();
Expand All @@ -345,7 +360,7 @@ mod tests {
}

// This seems to fail unpredictably on MacOs.
#[cfg(target_os = "linux")]
#[cfg(not(target_os = "macos"))]
#[test]
fn test_save_pdf() {
let test_plot = create_test_plot();
Expand Down

0 comments on commit 4973679

Please sign in to comment.