decrating: SPLIT shipper-storage — types to shipper-types, backend to shipper::ops::storage#68
Merged
Merged
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 25 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (41)
📒 Files selected for processing (20)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…age, backend → shipper::ops::storage Per docs/decrating-plan.md §3.2, shipper-storage is split by concern rather than absorbed wholesale. The crate currently only implements filesystem storage (S3/GCS/Azure bail out as "not yet implemented") — not a finished public product yet. This PR: - Moves StorageType, CloudStorageConfig, and their error types into shipper-types::storage (the stable contract surface that embedders use to declare their storage choice). anyhow is avoided here — the new ParseStorageTypeError and ValidateStorageConfigError keep the types crate free of anyhow. - Moves StorageBackend trait, FileStorage impl, cloud-backend stubs, build_storage_backend factory, and config_from_env env-parsing into shipper/src/ops/storage/ as crate-private (pub(crate)). We do not promise the StorageBackend trait publicly until cloud backends are real. - Introduces shipper::ops as the crate-private operational layer for future internals that should not yet be on the public API surface. - Updates shipper-config to import CloudStorageConfig/StorageType from shipper_types::storage and drops its shipper-storage dependency. - Removes the shipper-storage crate from the workspace and deletes it. - Updates docs/architecture.md and RELEASE_CHECKLIST_v0.3.0.md to drop shipper-storage references. Stacks on PR #53 (env+storage+git dedup intermediate). Per docs/decrating-plan.md §6 Phase 2 (storage SPLIT special case).
After cherry-picking PR #68's storage SPLIT commit onto main, `lib.rs` ended up with two `pub(crate) mod ops;` declarations: the original from main's ops/ layer introduction, and a new one added by PR #68 to house ops::storage. Removed the duplicate. Also preserved PR #66's per-call unique tmp-filename race fix in the new FileStorage::write location at `crates/shipper/src/ops/storage/mod.rs`. The pattern uses pid + thread-id + nanos suffix to prevent concurrent writes to the same destination from racing on rename.
ad5b1d6 to
784ff28
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Per
docs/decrating-plan.md§3.2,shipper-storageis split by concern rather than absorbed wholesale. The crate currently only implements filesystem storage (S3/GCS/Azure bail with "not yet implemented") — not a finished public product yet. Splitting keeps the stable contract surface inshipper-typesand the half-finished backend behindshipper's internals.Stacks on #53 (env+storage+git dual-impl cleanup intermediate).
What moved where
Config/data types →
shipper-types::storage(stable public contract)StorageType(enumFile | S3 | Gcs | Azure)CloudStorageConfigand all its buildersParseStorageTypeError,ValidateStorageConfigError(plain error structs — noanyhowdep inshipper-types)Runtime backend code →
shipper/src/ops/storage/(crate-privatepub(crate))StorageBackendtraitFileStoragefilesystem impl (atomic temp-file + rename)build_storage_backend(cfg)factoryconfig_from_env()env-var parserIntroduces
shipper::opsas the crate-private operational layer for internals not yet ready for public API.Callers updated
shipper-config::StorageConfigInnernow usesshipper_types::storage::{CloudStorageConfig, StorageType}.shipper-config's[dependencies]dropsshipper-storage.shipper's[dependencies]dropsshipper-storage.crates/shipper-storage.docs/architecture.mdandRELEASE_CHECKLIST_v0.3.0.mdstop listingshipper-storage.Why
StorageBackendis not publicpub(crate)lets us evolve the trait as cloud backends are implemented. When they are real, the module can be promoted to public or extracted into a new crate.LOC delta
shipper-types/src/storage/mod.rs: 554 LOC (types, tests, proptests, snapshot tests).shipper/src/ops/mod.rs+shipper/src/ops/storage/mod.rs: 600 LOC (trait, filesystem impl, stubs, tests).shipper-storage/crate: ~2040 LOC total (src/lib.rs 1664, tests/storage_integration.rs 375, Cargo.toml/README).Snapshots were renamed (insta picks up the new module path) —
git diff -Mshows most as renames.Test plan
cargo check --workspacepasses.cargo clippy --workspace --all-targets --all-features -- -D warningspasses.cargo fmt --all -- --checkpasses.cargo test --workspacepasses (all suites green, including the new 187-testshipper-typesrun and the 28-testshipper::ops::storagesuite).cargo insta acceptand committed.