The async-only direction was already the only one used in production:
* create_storage never accepted mode=sync; build_storage_config always
emitted mode=async; zero callers referenced StorageBackend / SyncData
/ S3StorageBackend.sync / LocalStorageBackend.sync anywhere.
* Drop the parallel sync base class, the sync concrete classes in
backends/local.py and backends/s3.py, and the boto3 dependency.
* Drop example_usage.py (zero importers; demonstration code, not part
of the public surface).
* Rename LocalAsyncStorageBackend -> LocalStorageBackend,
S3AsyncStorageBackend -> S3StorageBackend to reflect the single
remaining class per type.
* Tighten create_storage: any mode=... key now raises StorageConfigError
with the new pointer (settings.storage_backend controls behavior).
* Cleanup call sites: schedule.application.service.build_object_store
no longer passes mode=async to create_storage.
* Cosmetic touch-ups in backend/services/storage.py and
common/config.py docstrings where they still said "boto3" instead of
"S3 client".
Public API surface preserved: AsyncStorageBackend / ObjectMeta /
create_storage / build_storage_config / register_backend all keep
their names and call signatures. backend tests: 136 passed.
parent_object_id was never written or read by any application code (verified
via repo-wide grep: only the baseline migration and the ORM model referenced
it). The orphan index idx_storage_parent likewise served nothing.
Tree structure is maintained entirely via the materialized path in
storage_objects.relative_path (LIKE-prefix queries in
backend/src/backend/scripts.py: list_workspace_tree, list_workspace_directories).
The column mislead a prior review into proposing an adjacency-list table —
removing it eliminates that temptation for the next reader.
Migration: migrations/versions/f7a8b9c0d1e2_drop_storage_parent.py
- Drops idx_storage_parent first, then parent_object_id (correct order on MySQL).
- MySQL 8.0 has no IF EXISTS on DROP INDEX / DROP COLUMN, so calls are unconditional.
- Updates the storage_objects TABLE COMMENT so the materialized-path warning
reaches the DB, not just the ORM (Codex review finding).
- Downgrade restores both.
Model: common/src/common/db/models/storage.py
- Removes the dead Index entry and the dead column.
- Refreshes the table comment to flag the materialized-path contract.
Verified:
- alembic upgrade head: applied, head = f7a8b9c0d1e2
- SHOW INDEX / SHOW COLUMNS: 0 rows
- TABLE COMMENT updated in information_schema
- pytest backend/tests: 43 passed
The fix lands in three concentric layers, all backed by a single
INTERNAL_SERVICE_TOKEN shared secret so we have one mechanism
instead of three:
1. docker-compose: drop the backend.ports: 8891:8000 and
runtime.ports: 8892:8000 mappings. Nginx is the only host
ingress again (architecture §2.2).
2. /internal/v1/*: the storage control plane had six endpoints, five
of which were dead code (frontend already migrated to
/api/v1/data-resources/* with JWT; schedule only ever called
POST /internal/v1/objects). Delete the dead routes, mount the
one survivor with Depends(require_internal_service) that
compares the X-Internal-Service-Token header against
settings.internal_service_token with secrets.compare_digest.
3. POST /api/v1/jupyter on the runtime container: previously open
inside the Docker network. Same token mechanism — backend's
runtime_http_client now carries the header, runtime's
handle_jupyter_action requires the same header. /api/v1/health
stays open for the Nginx and compose healthchecks.
The schedule worker was already configured to call
POST /internal/v1/objects; build_storage_http_client now
sets the token header so its existing call site keeps working
without changes.
Files touched:
backend/src/backend/storage_api.py # 5 dead routes deleted + token guard
backend/src/backend/main.py # runtime_http_client header
runtime/src/runtime/main.py # require_internal_service Depends
common/src/common/config.py # internal_service_token setting
schedule/src/schedule/service.py # httpx client header
docker-compose.yml # ports dropped, INTERNAL_SERVICE_TOKEN env
.env.example # INTERNAL_SERVICE_TOKEN placeholder
API.md / README.md / DEVELOP.md # §9 trimmed to 1 endpoint
Verified:
compileall -> 0 errors
pytest backend/tests -> 37 passed
in-process ASGI smoke:
POST /internal/v1/objects no/wrong/correct token -> 401/401/200
POST /api/v1/jupyter no/wrong/correct token -> 401/401/200
5 deleted internal routes -> 404
docker compose config (with env) -> OK
P0-1 still has one open sub-item (rclone RC --rc-no-auth) that
the user has explicitly deferred; not touched here.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary of changes:
- Resources Bind Idempotency (High 1+2):
- Check existing active storage object bindings before duplicate check.
- Return existing binding (`reused: true`) on retry with same upload_id.
- Filter by `status == "active"` to bypass dead/deleted rows during reuse check.
- Storage Concurrent Overwrite (High 3):
- Add `acquire_named_lock` and `release_named_lock` helpers using MySQL `GET_LOCK`/`RELEASE_LOCK` hashed to <= 64 chars.
- Wrap `upload_bytes_to_session` PUT+INSERT critical section with named lock on `object_key`.
- Re-check key collision inside lock; append ULID suffix on collision.
- Shared Reference Deletion Protection (Medium 4):
- Check active references before deleting storage objects in `delete_resource`.
- Delete only `DataResources` record if storage object is still referenced elsewhere.
- Robust Usage Type Fallback (Medium 5):
- Replace direct dict lookup for `USAGE_TYPE_TO_PURPOSE[item.usage_type]` with `.get(..., "workspace")` default.
- Idempotency Key Path Matching (Medium 6):
- Move `file_name`/`target_path` validation forward and include path dimension in comparison.
- Strip uniqueness suffix via `_strip_uniqueness_suffix` before key comparison to avoid false 409s on valid retries.
- Usage Type & Bind Concurrency Control (Low 7 & 8):
- Reject bind requests with 409 if upload session purpose is not `data_resource`.
- Wrap resource duplicate check and creation in named lock using `(owner, directory, name)`.
- Trash Key Uniqueness & Restore Compatibility (Low 9):
- Update `trash_key` format to `{purpose}/{object_key}-{storage_object_id}` to prevent collisions.
- Update `object_key_hash` on trash move.
- Update restore logic in `storage_api.py` to strip suffix while maintaining backward compatibility with legacy keys.
- Dead Code Removal (Low 10):
- Remove unreachable `upload_status = "failed"` and redundant `session.rollback()` in `IntegrityError` block.
- Tests & Mocks:
- Add/update 5 test cases covering non-data_resource bind rejection, suffix stripping, and path recovery.
- Add named lock statement mocks for DB testing.
publish_version -> create_server_object_payload -> storage_payload reads
item.created_at on a sync helper. Without eager_defaults, server-default
columns stay unloaded after INSERT, the next sync read triggers a lazy
refresh through the async driver, and MissingGreenlet fires.
Enable mapper-level eager_defaults on StorageObjects so server-default
columns (created_at, updated_at, ...) are round-tripped into the ORM
object immediately after INSERT. No other table or session config
touched.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The factory now picks between two backends based on
settings.storage_backend ("s3" default, "local" for dev / single-node /
air-gapped deployments). The new factory helper build_storage_config()
takes one of the 4 PURPOSE_BUCKETS ("workspace" | "version" |
"run_log" | "trash") and returns the kwargs for create_storage(...).
s3 mode: AsyncStorageBackend over an S3-compatible service
(S3_WORKSPACE_BUCKET etc. as separate buckets).
local mode: AsyncStorageBackend over on-disk files; the 4 buckets
become subdirectories of LOCAL_STORAGE_BASE_DIR (default
"/data"), so the same 4-bucket layout works in both modes.
Concretely:
- common/config.py: add storage_backend (default "s3") +
local_storage_base_dir (default "/data").
- common/storage/factory.py: add PURPOSE_BUCKETS constant +
build_storage_config(bucket_name) helper.
- backend/main.py + backend/storage_api.py: lifespan collapses the
4-instance construction into one dict comprehension:
app.state.object_stores = {
name: create_storage(build_storage_config(name))
for name in PURPOSE_BUCKETS
}
(was 4x ~10-line dicts, one per bucket).
- runtime/mount.py: when STORAGE_BACKEND=local, skip the rclone mount
entirely (the shared docker volume at LOCAL_STORAGE_BASE_DIR is the
store; runtime reads directly).
- docker-compose.yml: mount the shared local-storage volume at /data
in both backend and runtime containers.
- .env.example: document STORAGE_BACKEND + LOCAL_STORAGE_BASE_DIR.
Dependencies added to support both backends:
- aiofiles>=25.1.0 (local async I/O) to backend + common + runtime.
- aioboto3>=15.5.0 (async S3) to common.
- uv.lock regenerated.
After this commit, both modes deploy end-to-end. The s3 mode is the
production default; local mode is opt-in via STORAGE_BACKEND=local.
Replace the old RustFS-specific storage layer (common.storage.client /
RustFSObjectStore) with a minimal sync/async abstraction:
AsyncStorageBackend: put / get / get_stream / delete / exists / stat /
list / get_url / copy
StorageBackend: same surface, sync implementations
create_storage({"type": "s3" | "local", "mode": "async", ...})
backends/s3.py: S3-compatible (boto3 / aioboto3)
backends/local.py: on-disk filesystem (aiofiles)
Concretely:
- Drop RustFSObjectStore + common.storage.client (deleted).
- Drop the RustFS-specific ensure_bucket / presign_put / move_to_trash /
rewrite_to_public_path / sha256 / put_bytes methods.
- Migrate backend/storage_api.py + backend/main.py + backend/scripts.py
+ schedule/service.py + schedule/worker.py to the new abstraction.
- Migrate backend/storage_client.py + schedule/storage_client.py to
stub status (HTTP wrapper is dead code post-migration; rewrite pending).
- Rename all RUSTFS_* env vars to S3_* across .env.example,
docker-compose.yml, default.conf, scripts/nginx-entrypoint.sh,
common/config.py.
- Replace hardcoded rclone remote name "rustfs" with "s3" in
docker-compose.yml + config.py default.
- Rename "rustfs" SQLAlchemy column comments + table comments to
provider-neutral wording; StorageObjects.storage_backend enum
value moves from "rustfs" to "s3" (DB rows with the old value will
fail the != "s3" check until a one-shot migration is applied).
- Drop unused common/src/common/migrations/{README,env.py,script.py.mako}
(the alembic setup lives in /migrations/, not here).
Migration of the old abstractions has been done in one pass; per-route
method calls (delete / stat / put / get_url) are now direct one-liners
against AsyncStorageBackend.
After this commit:
- All Python imports resolve; routes compile (compileall green).
- s3 mode is fully wired.
- Routes that depended on removed methods (presign_put, move_to_trash,
rewrite_to_public_path, head() metadata) raise NotImplementedError
with a one-line TODO; rewriting these route handlers is the next step.
schedule:
- New _execution_loop runs alongside _database_event_loop. It claims
job.node.execute rows, sets a 30-min lease on available_at, then
dispatches each as asyncio.create_task under a Semaphore(N).
Polling loop is back to sub-millisecond turnaround for
schedule.run.requested and job.node.finished. Long notebook
execution no longer blocks DAG advance events.
- _process_pending_events filters by event_type IN
('schedule.run.requested', 'job.node.finished'); the executor
loop owns job.node.execute exclusively.
- _process_outbox_event builds a plain dict envelope before
handler dispatch; the previous ORM-row handoff risked
DetachedInstanceError once the outer session closed.
- _sync_once uses get_job + reschedule_job for existing job ids
instead of add_job(replace_existing=True). Each cron schedule
no longer removed-and-readded every 5s.
- service.py threads settings.schedule_execution_concurrency into
the orchestrator (default 4).
common:
- create_async_engine gets explicit pool_size=10, max_overflow=20,
pool_recycle=1800. No more relying on SQLAlchemy defaults.
- New schedule_execution_concurrency setting.
runtime:
- scan_workspaces: add missing 'import os' (NameError on startup)
and switch to asyncio.gather bounded by Semaphore(4) so N
workspaces start in parallel instead of sequentially.
Co-Authored-By: Claude <noreply@anthropic.com>
- scripts.update_script: repoint script.current_object_id to the
newly uploaded StorageObject and best-effort delete the old
working copy. Previously the old row was mutated with the new
content_hash while the script still pointed at it, silently
losing user edits on the next publish_version.
- scripts.publish_version: read object bytes via the new
RustFSObjectStore.get_bytes() instead of the non-existent
.get_object(); publish 500'd on every call.
- scripts.upload_script / storage_api.complete_upload_record:
Path(file_name) raised NameError (only PurePosixPath imported),
crashing every upload and every upload finalization. Use
PurePosixPath.
- RustFSObjectStore: add get_bytes() helper (sync, body.close in
finally) for in-process callers that need raw bytes.
Co-Authored-By: Claude <noreply@anthropic.com>
- common.config.Settings: pydantic-settings with @lru_cache singleton;
all env vars now declared in one place (database / JWT / RUSTFS_*
credentials + 3 purpose-named buckets / workspace FS roots / etc.).
Replaces os.environ / os.getenv in backend / schedule / runtime /
common modules.
- storage_api: object_key layout flattens from
"{ws}/{usage_type}/{ulid}/{name}" to "{ws}/{ulid}". File name, type,
and logical path live in the StorageObjects / Scripts row, not in
the S3 key, so the bucket can be re-organised without a DB rewrite.
- storage_api: new BUCKET_FOR_USAGE map and resolve_bucket() helper
route uploads by usage_type to the right purpose-named bucket:
working_copy / public_script / data_resource / snapshot
→ RUSTFS_WORKSPACE_BUCKET (workspaces)
version_artifact
→ RUSTFS_VERSION_BUCKET (versions)
run_log / run_result
→ RUSTFS_RUN_LOG_BUCKET (run-logs)
workspace.artifact_bucket override wins over the default for that
workspace. Unknown usage_type falls through to the workspace bucket
so uploads are never silently dropped.
- backend.main lifespan: ensure_bucket loops over all three buckets at
startup.
- common.storage.schemas: extend usage_type Literal to include
working_copy / public_script (consumed by scripts.py after the local
FS removal).
- common.storage.client: raise StorageClientError / StorageUnavailable /
StorageRequestFailed instead of FastAPI HTTPException, so the client
is usable from non-FastAPI contexts (e.g. schedule worker). The
register_workspace_object method is removed (the local-FS path it
routed to no longer exists).
- common.pyproject.toml: add greenlet>=3.0.0 (SQLAlchemy 2.0 async
engine.dispose() requires it) and pydantic-settings>=2.14.2.
Verified: backend.main 57 routes; docker compose config; 20 SQLAlchemy
tables, 0 ForeignKey; grep os.environ / os.getenv in
backend|schedule|runtime|common = 0.
tao.chenandtao.chen