Skip to content

Commit 24ea40a

Browse files
committed
refactor and cleanup
- cleanup build.rs - fix offline mode functionality: was erroring because there are size restrictions when using data-uri data, cannot load large data-uri files, so for offline mode, default to to_file Signed-off-by: Andrei Gherghescu <[email protected]> wip Signed-off-by: Andrei Gherghescu <[email protected]>
1 parent 22e3d78 commit 24ea40a

File tree

8 files changed

+119
-148
lines changed

8 files changed

+119
-148
lines changed

.github/scripts/setup-windows-static-export.ps1

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,15 @@ Write-Host "=== Setting up Windows environment for static export ==="
1313
$chromedriverPath = $ChromeDriverPath
1414
if (-not (Test-Path $chromedriverPath)) {
1515
Write-Host "Action output chromedriver path not found, searching for alternatives..."
16-
16+
1717
$commonPaths = @(
1818
"C:\Program Files\Google\Chrome\Application\chromedriver.exe",
1919
"C:\Program Files (x86)\Google\Chrome\Application\chromedriver.exe",
2020
"$env:USERPROFILE\AppData\Local\Google\Chrome\Application\chromedriver.exe",
2121
"$env:PROGRAMFILES\Google\Chrome\Application\chromedriver.exe",
2222
"$env:PROGRAMFILES(X86)\Google\Chrome\Application\chromedriver.exe"
2323
)
24-
24+
2525
foreach ($path in $commonPaths) {
2626
if (Test-Path $path) {
2727
Write-Host "Using chromedriver from: $path"
@@ -60,14 +60,14 @@ if (-not (Test-Path $chromePath)) {
6060

6161
# Set environment variables
6262
$env:WEBDRIVER_PATH = $chromedriverPath
63-
$env:CHROME_PATH = $chromePath
63+
$env:BROWSER_PATH = $chromePath
6464
$env:RUST_LOG = "debug"
6565
$env:RUST_BACKTRACE = "1"
6666
$env:ANGLE_DEFAULT_PLATFORM = "swiftshader"
6767

6868
Write-Host "Environment variables set:"
6969
Write-Host "WEBDRIVER_PATH: $env:WEBDRIVER_PATH"
70-
Write-Host "CHROME_PATH: $env:CHROME_PATH"
70+
Write-Host "BROWSER_PATH: $env:BROWSER_PATH"
7171
Write-Host "RUST_LOG: $env:RUST_LOG"
7272
Write-Host "RUST_BACKTRACE: $env:RUST_BACKTRACE"
7373

@@ -80,14 +80,14 @@ if (-not (Test-Path $env:WEBDRIVER_PATH)) {
8080
exit 1
8181
}
8282

83-
if (-not (Test-Path $env:CHROME_PATH)) {
84-
Write-Error "Chrome not found at: $env:CHROME_PATH"
83+
if (-not (Test-Path $env:BROWSER_PATH)) {
84+
Write-Error "Chrome not found at: $env:BROWSER_PATH"
8585
exit 1
8686
}
8787

8888
# Test Chrome version
8989
try {
90-
$chromeVersion = & "$env:CHROME_PATH" --version 2>&1
90+
$chromeVersion = & "$env:BROWSER_PATH" --version 2>&1
9191
Write-Host "Chrome version: $chromeVersion"
9292
} catch {
9393
Write-Host "Failed to get Chrome version: $_"
@@ -101,4 +101,4 @@ try {
101101
Write-Host "Failed to get chromedriver version: $_"
102102
}
103103

104-
Write-Host "=== Windows environment setup completed ==="
104+
Write-Host "=== Windows environment setup completed ==="

.github/workflows/ci.yml

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,11 @@ jobs:
8080
chrome-version: 'latest'
8181
install-chromedriver: true
8282
id: setup-chrome
83-
83+
8484
- uses: actions/checkout@v4
85-
85+
8686
- uses: dtolnay/rust-toolchain@stable
87-
87+
8888
# Cache cargo registry for all platforms
8989
- name: Cache cargo registry
9090
uses: actions/cache@v4
@@ -96,7 +96,7 @@ jobs:
9696
key: ${{ runner.os }}-cargo-registry-${{ hashFiles('**/Cargo.lock') }}
9797
restore-keys: |
9898
${{ runner.os }}-cargo-registry-
99-
99+
100100
# Windows-specific environment setup for static export tests
101101
- name: Setup Windows environment for static export
102102
if: matrix.os == 'windows-latest'
@@ -105,30 +105,30 @@ jobs:
105105
-ChromeVersion "${{ steps.setup-chrome.outputs.chrome-version }}" `
106106
-ChromePath "${{ steps.setup-chrome.outputs.chrome-path }}" `
107107
-ChromeDriverPath "${{ steps.setup-chrome.outputs.chromedriver-path }}"
108-
108+
109109
# Run tests on non-Windows platforms
110-
- name: Run tests (${{ matrix.os }})
110+
- name: Run tests (${{ matrix.os }})
111111
if: matrix.os != 'windows-latest'
112112
run: cargo test --verbose --workspace --features plotly_ndarray,plotly_image,static_export_default --exclude plotly_kaleido
113-
113+
114114
# Run tests on Windows with Chrome WebDriver
115-
- name: Run tests (${{ matrix.os }})
115+
- name: Run tests (${{ matrix.os }})
116116
if: matrix.os == 'windows-latest'
117117
shell: pwsh
118118
run: |
119119
# Set environment variables for WebDriver
120120
$env:WEBDRIVER_PATH = "${{ steps.setup-chrome.outputs.chromedriver-path }}"
121-
$env:CHROME_PATH = "${{ steps.setup-chrome.outputs.chrome-path }}"
121+
$env:BROWSER_PATH = "${{ steps.setup-chrome.outputs.chrome-path }}"
122122
$env:RUST_LOG = "debug"
123123
$env:RUST_BACKTRACE = "1"
124124
$env:ANGLE_DEFAULT_PLATFORM = "swiftshader"
125-
125+
126126
Write-Host "Environment variables set:"
127127
Write-Host "WEBDRIVER_PATH: $env:WEBDRIVER_PATH"
128-
Write-Host "CHROME_PATH: $env:CHROME_PATH"
129-
128+
Write-Host "BROWSER_PATH: $env:BROWSER_PATH"
129+
130130
cargo test --verbose --workspace --features plotly_ndarray,plotly_image,static_export_chromedriver --exclude plotly_kaleido
131-
131+
132132
code-coverage:
133133
name: Code Coverage
134134
runs-on: ubuntu-latest
@@ -165,14 +165,14 @@ jobs:
165165
runs-on: ubuntu-latest
166166
steps:
167167
- uses: actions/checkout@v4
168-
168+
169169
- name: Setup Chrome (for static_export)
170170
if: matrix.example == 'static_export'
171171
uses: browser-actions/setup-chrome@v1
172172
with:
173173
chrome-version: 'latest'
174174
install-chromedriver: true
175-
175+
176176
- uses: dtolnay/rust-toolchain@stable
177177
- run: cd ${{ github.workspace }}/examples/${{ matrix.example }} && cargo build
178178

@@ -193,18 +193,18 @@ jobs:
193193

194194

195195
build_book:
196-
name: Build Book
196+
name: Build Book
197197
runs-on: ubuntu-latest
198198
steps:
199199
- uses: actions/checkout@v4
200-
200+
201201
- name: Setup Chrome (for static_export)
202202
uses: browser-actions/setup-chrome@v1
203203
with:
204204
chrome-version: 'latest'
205205
install-chromedriver: true
206206
id: setup-chrome
207-
207+
208208
- uses: dtolnay/rust-toolchain@stable
209209
- run: cargo install mdbook --no-default-features --features search --vers "^0.4" --locked --quiet
210210
- name: Build examples to generate needed html files
@@ -218,5 +218,5 @@ jobs:
218218
cd ${{ github.workspace }}/examples/shapes && cargo run
219219
cd ${{ github.workspace }}/examples/themes && cargo run
220220
cd ${{ github.workspace }}/examples/static_export && cargo run
221-
- name: Build book
221+
- name: Build book
222222
run: mdbook build docs/book

plotly_static/build.rs

Lines changed: 27 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -21,24 +21,23 @@ const DRIVER_EXT: &str = ".exe";
2121
#[cfg(not(target_os = "windows"))]
2222
const DRIVER_EXT: &str = "";
2323

24-
const CHROME_PATH_ENV: &str = "CHROME_PATH";
25-
const CHROMEDRIVER_PATH_ENV: &str = "CHROMEDRIVER_PATH";
26-
const FIREFOX_PATH_ENV: &str = "FIREFOX_PATH";
27-
const GECKODRIVER_PATH_ENV: &str = "GECKODRIVER_PATH";
28-
const INSTALL_PATH_ENV: &str = "INSTALL_BIN_PATH";
24+
const BROWSER_BIN_PATH_ENV: &str = "BROWSER_PATH";
25+
const WEBDRIVER_BIN_PATH_ENV: &str = "WEBDRIVER_PATH";
26+
const INSTALL_PATH_ENV: &str = "WEBDRIVER_INSTALL_PATH";
27+
const GECKODRIVER_NAME: &str = "geckodriver";
28+
const CHROMEDRIVER_NAME: &str = "chromedriver";
2929

3030
const MAX_DOWNLOAD_RETRIES: u32 = 3;
3131
const INITIAL_RETRY_DELAY: u64 = 2;
3232

3333
struct WebdriverDownloadConfig {
3434
driver_name: &'static str,
35-
path_env: &'static str,
3635
get_browser_path: fn() -> Result<PathBuf>,
3736
}
3837

3938
/// Get user's bin directory for driver installs (e.g., $HOME/.local/bin or
4039
/// %USERPROFILE%\.local\bin) or set it to the one specified via the ENV
41-
/// variable `INSTALL_BIN_PATH`
40+
/// variable `WEBDRIVER_INSTALL_PATH`
4241
fn user_bin_dir() -> PathBuf {
4342
if let Ok(bin) = env::var(INSTALL_PATH_ENV) {
4443
return PathBuf::from(bin);
@@ -58,17 +57,17 @@ fn user_bin_dir() -> PathBuf {
5857

5958
/// Check if a driver is already installed at the given path from environment
6059
/// variable
61-
fn is_webdriver_available(env_var: &str, bin_name: &str) -> bool {
60+
fn is_webdriver_available(bin_name: &str) -> bool {
6261
// First check environment variable path
63-
if let Ok(path) = env::var(env_var) {
62+
if let Ok(path) = env::var(WEBDRIVER_BIN_PATH_ENV) {
6463
let bin_path = if cfg!(target_os = "windows") && !path.to_lowercase().ends_with(".exe") {
6564
format!("{path}{DRIVER_EXT}")
6665
} else {
6766
path
6867
};
6968
let exe = Path::new(&bin_path);
7069
if exe.exists() && exe.is_file() {
71-
println!("{bin_name} found at path specified in {env_var}: {bin_path}");
70+
println!("{bin_name} found at path specified in {WEBDRIVER_BIN_PATH_ENV}: {bin_path}");
7271
return true;
7372
}
7473
}
@@ -121,21 +120,19 @@ async fn download_with_retry(
121120
}
122121

123122
Err(anyhow!(
124-
"Failed to download driver after {} attempts: {:?}",
125-
MAX_DOWNLOAD_RETRIES,
126-
last_error
123+
"Failed to download driver after {MAX_DOWNLOAD_RETRIES} attempts: {last_error:?}",
127124
))
128125
}
129126

130127
fn setup_driver(config: &WebdriverDownloadConfig) -> Result<()> {
131-
if is_webdriver_available(config.path_env, config.driver_name) {
128+
if is_webdriver_available(config.driver_name) {
132129
return Ok(());
133130
}
134131
println!(
135-
"cargo::warning=You can specify {GECKODRIVER_PATH_ENV} or {CHROMEDRIVER_PATH_ENV} to an existing installation to avoid downloads."
132+
"cargo::warning=You can specify {WEBDRIVER_BIN_PATH_ENV} to an existing {CHROMEDRIVER_NAME}/{GECKODRIVER_NAME} installation to avoid downloads."
136133
);
137134
println!(
138-
"cargo::warning=You can override browser detection using {CHROME_PATH_ENV} or {FIREFOX_PATH_ENV} environment variables."
135+
"cargo::warning=You can override browser detection using {BROWSER_BIN_PATH_ENV} environment variable."
139136
);
140137

141138
println!(
@@ -175,15 +172,15 @@ fn setup_driver(config: &WebdriverDownloadConfig) -> Result<()> {
175172
.context("Failed to create Tokio runtime")?;
176173

177174
match config.driver_name {
178-
"chromedriver" => {
175+
CHROMEDRIVER_NAME => {
179176
let driver_info = ChromedriverInfo::new(webdriver_bin.clone(), browser_path);
180177
runtime
181178
.block_on(async { download_with_retry(&driver_info, false, true, 1).await })
182179
.with_context(|| {
183180
format!("Failed to download and install {}", config.driver_name)
184181
})?;
185182
}
186-
"geckodriver" => {
183+
GECKODRIVER_NAME => {
187184
let driver_info = GeckodriverInfo::new(webdriver_bin.clone(), browser_path);
188185
runtime
189186
.block_on(async { download_with_retry(&driver_info, false, true, 1).await })
@@ -204,13 +201,13 @@ fn setup_driver(config: &WebdriverDownloadConfig) -> Result<()> {
204201

205202
#[cfg(feature = "chromedriver")]
206203
fn get_chrome_path() -> Result<PathBuf> {
207-
if let Ok(chrome_path) = env::var(CHROME_PATH_ENV) {
204+
if let Ok(chrome_path) = env::var(BROWSER_BIN_PATH_ENV) {
208205
let path = PathBuf::from(&chrome_path);
209206
if path.exists() {
210207
Ok(path)
211208
} else {
212-
Err(anyhow!("Chrome not found on path: {}", chrome_path)).with_context(|| {
213-
format!("Please set {CHROME_PATH_ENV} to a valid Chrome installation")
209+
Err(anyhow!("Chrome not found on path: {chrome_path}")).with_context(|| {
210+
format!("Please set {BROWSER_BIN_PATH_ENV} to a valid Chrome installation")
214211
})
215212
}
216213
} else {
@@ -222,24 +219,21 @@ fn get_chrome_path() -> Result<PathBuf> {
222219
Ok(old_browser_path)
223220
} else {
224221
Err(anyhow!("Chrome browser not detected")).with_context(|| {
225-
format!("Use {CHROME_PATH_ENV} to point to a valid Chrome installation")
222+
format!("Use {BROWSER_BIN_PATH_ENV} to point to a valid Chrome installation")
226223
})
227224
}
228225
}
229226
}
230227

231228
#[cfg(feature = "geckodriver")]
232229
fn get_firefox_path() -> Result<PathBuf> {
233-
if let Ok(firefox_path) = env::var(FIREFOX_PATH_ENV) {
230+
if let Ok(firefox_path) = env::var(BROWSER_BIN_PATH_ENV) {
234231
let path = PathBuf::from(firefox_path.clone());
235232
if path.exists() {
236233
Ok(path)
237234
} else {
238235
Err(anyhow!("Firefox not found on path: {firefox_path}")).with_context(|| {
239-
format!(
240-
"Please set {} to a valid Firefox installation",
241-
FIREFOX_PATH_ENV
242-
)
236+
format!("Please set {BROWSER_BIN_PATH_ENV} to a valid Firefox installation",)
243237
})
244238
}
245239
} else {
@@ -248,10 +242,7 @@ fn get_firefox_path() -> Result<PathBuf> {
248242
Ok(browser_path)
249243
} else {
250244
Err(anyhow!("Firefox browser not detected")).with_context(|| {
251-
format!(
252-
"Use {} to point to a valid Firefox installation",
253-
FIREFOX_PATH_ENV
254-
)
245+
format!("Use {BROWSER_BIN_PATH_ENV} to point to a valid Firefox installation",)
255246
})
256247
}
257248
}
@@ -295,7 +286,7 @@ async fn download(
295286
driver_info.download_verify_install(num_tries).await?;
296287
}
297288

298-
println!("cargo::warning=Driver installed succesfully ...");
289+
println!("cargo::warning=Driver installed successfully ...");
299290
Ok(())
300291
}
301292
}
@@ -312,8 +303,7 @@ fn main() -> Result<()> {
312303
#[cfg(feature = "chromedriver")]
313304
{
314305
let config = WebdriverDownloadConfig {
315-
driver_name: "chromedriver",
316-
path_env: CHROMEDRIVER_PATH_ENV,
306+
driver_name: CHROMEDRIVER_NAME,
317307
get_browser_path: get_chrome_path,
318308
};
319309
setup_driver(&config)?;
@@ -322,8 +312,7 @@ fn main() -> Result<()> {
322312
#[cfg(feature = "geckodriver")]
323313
{
324314
let config = WebdriverDownloadConfig {
325-
driver_name: "geckodriver",
326-
path_env: GECKODRIVER_PATH_ENV,
315+
driver_name: GECKODRIVER_NAME,
327316
get_browser_path: get_firefox_path,
328317
};
329318
setup_driver(&config)?;
@@ -336,12 +325,12 @@ fn main() -> Result<()> {
336325
} else {
337326
#[cfg(feature = "chromedriver")]
338327
{
339-
let msg = "'webdriver_download' feature disabled. Please install a 'chromedriver' version manually and make the environment variable 'WEBDRIVER_PATH' point to it.".to_string();
328+
let msg = format!("'webdriver_download' feature disabled. Please install a '{CHROMEDRIVER_NAME}' version manually and make the environment variable 'WEBDRIVER_PATH' point to it.");
340329
println!("cargo::warning={msg}");
341330
}
342331
#[cfg(feature = "geckodriver")]
343332
{
344-
let msg = format!("'webdriver_download' feature disabled. Please install a 'geckodriver' version manually and make the environment variable 'WEBDRIVER_PATH' point to it.");
333+
let msg = format!("'webdriver_download' feature disabled. Please install a '{GECKODRIVER_NAME}' version manually and make the environment variable 'WEBDRIVER_PATH' point to it.");
345334
println!("cargo::warning={msg}");
346335
}
347336
#[cfg(not(any(feature = "chromedriver", feature = "geckodriver")))]
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use serde_json::Value;
1313
#[command(version)]
1414
struct Cli {
1515
/// Input file containing Plotly JSON (use '-' for stdin)
16-
#[arg(short, long, default_value = "-")]
16+
#[arg(short, long, required = true, default_value = "-")]
1717
input: String,
1818

1919
/// Output file path

plotly_static/examples/test_chrome_binary.rs renamed to plotly_static/examples/test_chrome_path.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ use plotly_static::{ImageFormat, StaticExporterBuilder};
22
use serde_json::json;
33

44
fn main() {
5-
// Set CHROME_PATH to test the binary capability
6-
std::env::set_var("CHROME_PATH", "/usr/bin/google-chrome");
5+
// Set BROWSER_PATH to test the binary capability
6+
std::env::set_var("BROWSER_PATH", "/usr/bin/google-chrome");
77

88
let plot = json!({
99
"data": [{

0 commit comments

Comments
 (0)