Skip to content

refactor: deduplicate shared utilities across driver crates#1660

Merged
TaylorMutch merged 1 commit into
NVIDIA:mainfrom
ericcurtin:refactor-dedup-shared-utilities/ec
Jun 1, 2026
Merged

refactor: deduplicate shared utilities across driver crates#1660
TaylorMutch merged 1 commit into
NVIDIA:mainfrom
ericcurtin:refactor-dedup-shared-utilities/ec

Conversation

@ericcurtin
Copy link
Copy Markdown
Contributor

Summary

Remove three sets of duplicated code by consolidating definitions into `openshell-core` where all consumers already have a dependency.

Related Issue

N/A — housekeeping refactor.

Changes

  • `format_bytes` — identical 14-line byte-formatting function existed independently in the Docker, Kubernetes, and VM drivers. Moved to `openshell-core::progress` (where all three already imported other symbols) and removed the three local copies.
  • `DEFAULT_SANDBOX_PIDS_LIMIT` — the `i64` constant `2048` was defined separately in the Docker driver and Podman config. Moved to `openshell-core::config` alongside the other shared defaults (`DEFAULT_STOP_TIMEOUT_SECS`, etc.). Podman re-exports it via `pub use` so existing intra-crate call sites continue to compile without changes.
  • `current_time_ms` — `openshell-sandbox::secrets` reimplemented the same logic already provided by `openshell-core::time::now_ms()`. Removed the local copy and called `now_ms()` directly.

Net: −31 lines, 0 behaviour change.

Testing

  • `cargo build --workspace` passes
  • `cargo test --workspace --exclude openshell-server` — all tests pass (797 unit tests across the workspace)
  • E2E tests added/updated (not applicable — no behaviour change)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)

Move three duplicated definitions into openshell-core so every
consumer has a single canonical source:

- format_bytes: identical 14-line function existed in docker,
  kubernetes, and vm drivers. Moved to openshell-core::progress
  where all three already imported from.

- DEFAULT_SANDBOX_PIDS_LIMIT: i64 constant (2048) duplicated in
  docker driver and podman config. Moved to openshell-core::config
  alongside other shared defaults. Podman re-exports it for
  internal call-site compatibility.

- current_time_ms: secrets.rs in openshell-sandbox reimplemented
  the same logic as openshell-core::time::now_ms(). Remove the
  local copy and call now_ms() directly via the existing dep.
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Jun 1, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@mrunalp
Copy link
Copy Markdown
Collaborator

mrunalp commented Jun 1, 2026

/ok to test b2519ca

@TaylorMutch TaylorMutch merged commit 29e2539 into NVIDIA:main Jun 1, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants