Develop #16
Assignees
tao.chen (tao chen)
Clear assignees
tao.chen (tao chen)
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: tao.chen/model-platform#16
Reference in New Issue
Block a user
- 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.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>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.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.All operator- and developer-facing docs updated to reflect: - The unified AsyncStorageBackend abstraction (s3 + local backends). - The STORAGE_BACKEND toggle ("s3" default, "local" for dev / single-node / air-gapped deployments). - The 4-purpose-bucket layout (workspace / version / run_log / trash) in both modes — 4 separate S3 buckets in s3 mode, 4 subdirectories of LOCAL_STORAGE_BASE_DIR in local mode. - The S3_* env var naming (was RUSTFS_*). - The server-proxied upload flow (was browser-direct presign-PUT): POST /internal/v1/uploads → PUT /internal/v1/uploads/{id} with raw bytes → server calls backend.put(). - The factory helpers workspaces_root() (runtime's view of the workspace bucket on disk) and rclone_remote_spec() (s3-mode mount source). - The "two settings describing the same thing" cleanup: the deleted settings.workspace_root, settings.workspaces_root, and settings.remote_bucket fields. Files touched: - API.md (§5 data-resource upload flow, §9 storage control plane, §10 readiness example) - ARCHITECTURE.md (storage layer diagram) - CLAUDE.md (architecture description + volume-preservation note) - DEVELOP.md (settings list, Storage section, "Wire a new bucket" how-to, dev-export example, troubleshooting network hint) - README.md (architecture diagram, container table, quick-start credentials note, tear-down note, Storage layout section) - REFACTOR_NOTES.md (final container list with s3 explanation) - backend/README.md (storage backend description) - migrations/data/README.md (step 11/12 record mentioning object storage) A handful of historical "RustFS" mentions are intentionally retained where they name a specific S3-compatible product (e.g. as an example in REFACTOR_NOTES.md's container list) or document the pre-2026 abstraction name (DEVELOP.md Storage section).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.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>B1: `_mark_upload_failed_and_raise` now commits on a separate session - Helper takes `request + upload_id`, opens a fresh session from `request.app.state.session_factory` and commits there before raising. - Closes the named-lock connection-pool leak Codex flagged: the old "commit-on-the-same-session" implementation could return the GET_LOCK connection to the pool before the enclosing `finally: release_named_lock` ran, leaking `mp:<hash>` for up to `pool_recycle` and re-opening the same-key upload race. - Same helper now used by `create_server_object_payload`'s put-failure branch — two failure paths have identical semantics. B2: streaming copy for soft-delete + restore (`get_stream() + put()`) - LOCAL backend: zero-copy (aiofiles stream write). OOM fixed. - S3 backend: still OOMs on multi-GB objects — `put()` materializes the async iter via `b"".join(chunks)`. Multipart `put` is a follow-up; do NOT claim "OOM fixed on production" since production defaults to S3. C1: worker re-verifies `Users.status='active' AND is_deleted=0` - `_assert_user_active` called from `_execution_context` after resolving `triggered_by`; skips `SYSTEM_CRON_USER_ID`. - `USER_DISABLED` error_code goes into the `NODE_FINISHED_EVENT` outbox payload — `schedule_node_runs` has no `error_code` column, the row only carries the `message` text. Docstrings corrected to say so explicitly (previous docstring falsely promised row-level observability). F1: honest browser-local file lock - `api.ts` `acquireFileLock/heartbeatFileLock/releaseFileLock/ releaseFileLockOnUnload` are now no-ops with comments stating they never call the network. - `scriptWorkspaceStore` dropped `tickHeartbeats`; `tickCleanup` simplified to just clear cache. - `useEditSessionLifecycle` dropped its 15s heartbeat `setInterval`. - `ScriptWorkspace.tsx` renders `.local-lock-banner` info bar when `isEditing`. Two tabs may still silently last-write — banner is the only guard (acceptable disclosure-only tradeoff). Dead code: deleted the duplicate `upload_bytes_to_session` in `backend/src/backend/storage_api.py`. The `services.storage` import is now the only source of the function; `create_upload_record`'s docstring updated to point at `backend.resources`.