Repository navigation
Conversation
DVC never worked: uploaded files were never under DVC_REPO_PATH, so every DVC call raised and createMajorVersion always failed. Remove the manager, the setup command, version-change detection and the post_save signal. createMajorVersion now writes the ResourceVersion row directly inside transaction.atomic(), behind IsAuthenticated plus a check that the caller owns the dataset or may change it. BREAKING CHANGE: the setup_dvc management command and DVC_* settings are gone.
Media moves to S3 only, through django-storages. Two storages share one bucket under <STORAGE_ENV>/public and <STORAGE_ENV>/private: public assets get unsigned, cacheable URLs; private files get 5-minute presigned URLs. The default storage is private, so anything that falls back to it is never world-readable by accident. Empty access keys use the boto3 credential chain (ECS/EC2 task role). boto/urllib3 loggers are capped at WARNING because at DEBUG they log every signed request, Authorization header included. Tests swap all storages for InMemoryStorage and never reach S3.
Every FileField/ImageField now names a storage: logos, cover images, profile pictures and publication block files go to public storage; resource files and chart images go to private storage. The storages are callables, so the migrations record a function reference, not bucket settings. upload_to paths drop their "files/public/" and "files/" prefixes, which only repeated what MEDIA_ROOT used to add; the storages already put every key under <env>/public or <env>/private.
S3 has no FieldFile.path, so every reader that used it would raise NotImplementedError once media left local disk. - local_copy() yields a temporary local copy (keeping the extension) for pandas, libmagic and the YAML/XML parsers; data indexing, entry counts and MIME validation use it. - The lru_cache on the loaders is dropped: it keyed on temp paths that are deleted on exit, so it only held DataFrames in memory. - Downloads stream with FileResponse instead of reading whole objects. - Charts only check that the file exists; they read the indexed table. - Search documents index the storage URL instead of a rewritten disk path.
Links issued before the move pointed at /public/<name> on disk. That route now redirects to the file's public S3 URL, after stripping the old "files/public/" or "files/" prefix from the name. Only public files redirect. The old static() route also served resources and drafts whenever DEBUG was on; anything outside public storage, or a name that tries to traverse paths, is a 404 here.
Rows created on disk carry a "files/public/" or "files/" prefix that the S3 keys no longer use. For each stored name the command puts the file at the normalised key in the storage its field names, taken from the old S3 key or, with --source, from the old disk tree, then repoints the row. Dry run by default; --execute copies and renames. Safe to re-run (names already in place are skipped) and deletes nothing.
Regression tests for the local-disk/DVC to S3 migration: S3 storage options (unsigned public URLs, 5-minute presigned private URLs, private default), which storage each file field names, local_copy() and MIME checks on storage without paths, prefix-free upload names, the legacy /public redirect (public files only, no traversal), createMajorVersion's owner-only version write, and migrate_media_to_s3 (dry run, disk and old S3 sources, missing files, failed copies).
Replace the DVC references in SECURITY.md and AGENTS.md with the S3 storage layout and access rules, and add the no-co-author commit rule to AGENTS.md.
Agents kept tripping over setup the file never mentioned (.env and logs/ are needed before tests boot; pre-commit pins python3.11) and over S3-era rules that lived only in code review. Add a quick start, a commands table with the SDK test command, task checklists for mutations, model changes and bug fixes, a files-and-media section (no FieldFile.path, local_copy, FileResponse, public vs private storage), and git safety rules (self-contained commits, no AI co-author trailer, never stash the user's work). Existing rules are unchanged.
This branch has not been deployed
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
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
Uploaded media now lives only in S3, through
django-storages, and the broken DVC versioning is removed.Several code paths read uploads with
FieldFile.path: data indexing, upload MIME validation, search documents and DVC. S3 has no local path, so each of those would fail once media moved to a bucket. DVC never worked at all: files were never underDVC_REPO_PATH, socreateMajorVersionalways failed.Changes
<STORAGE_ENV>/publicand<STORAGE_ENV>/private.FileField/ImageFieldnames its storage (migrationsapi 0054,authorization 0005). Upload paths drop the oldfiles/public/andfiles/prefixes.local_copy()gives pandas, libmagic and the YAML/XML parsers a temporary local file. Downloads stream withFileResponse. Search documents index the storage URL./public/<name>redirects to the file's S3 URL, for public files only. Private files and path-traversal attempts get a 404. The oldstatic()route served drafts and resources whenever DEBUG was on.setup_dvccommand, version detection, the post-save signal and theDVC_*settings are gone.createMajorVersionnow writes the version row insidetransaction.atomic(), and requires the caller to be authenticated and to own the dataset or be allowed to change it.migrate_media_to_s3: a new command that copies existing files to their new names, taking them from the old disk tree or the old S3 keys, and repoints each row. It is a dry run unless you pass--execute, it is safe to re-run, and it deletes nothing.SECURITY.mdandAGENTS.mdare updated for the new storage, andAGENTS.mdgains setup steps and task checklists.STORAGE_BUCKET,STORAGE_REGIONandSTORAGE_ENV, plus optionallySTORAGE_PUBLIC_BASE_URL(see.env.example). Settings fail to load without the first three.STORAGE_ACCESS_KEY_ID/STORAGE_SECRET_ACCESS_KEYempty on ECS/EC2 so the task role is used. The role needs read, write and delete on the bucket.*/public/*, which the bucket policy makes readable. Object ACLs are not used.python manage.py migrate.python manage.py migrate_media_to_s3 --source /code/files/public(a dry run), review the counts, then run it again with--execute.setup_dvcand theDVC_*variables are gone, and so is serving media from local disk.Testing
pytestwith the CI flags: 265 passed. New regression tests are intests/test_s3_storage.py.pre-commititself couldn't run locally because its hooks needpython3.11.