SUBMISSION-169: Ensure that call to preflight is using a fresh tar archive.#76
Open
DavidLFielding wants to merge 8 commits into
Open
SUBMISSION-169: Ensure that call to preflight is using a fresh tar archive.#76DavidLFielding wants to merge 8 commits into
DavidLFielding wants to merge 8 commits into
Conversation
…eflight and preview. Stale user_decisions was feeding into the next directives regeneration, so Review Files showed the old file list even after preflight was correctly re-run. Full rationale in the _common_file_change_execute docstring. [SUBMISSION-169] David
… The persisted <id>.tar.gz needs to be rebuilt from current src/ before each preflight call (otherwise tex2pdf scans a snapshot from a previous compile) and deleted on every file change (so it can't linger as stale). build_source_package returns bytes for the Download Source Package controller; write_source_package builds and uploads to the canonical path for start_preflight; delete_source_package removes the path for invalidation. Concrete implementations follow. [SUBMISSION-169] David
…e_package walks the workspace, downloads each blob, and packs them into an in-memory gzipped tarball; missing or unreadable blobs are skipped with a warning rather than aborting (matches the old _build_source_tarball semantics that this replaces). write_source_package builds and uploads to the canonical <id>.tar.gz path. delete_source_package removes that path if present. Empty workspaces yield a valid empty gzipped tar. Logs files_added/skipped/size_bytes so any rebuild failure is visible in the dev log. [SUBMISSION-169] David
…ract methods. NullFileStore is used as the default in NullImplementation and as a parent for MockFileStore; without these stubs neither can be instantiated after the FileStore interface gained the source-package methods. [SUBMISSION-169] David
…le dict. build_source_package re-tars the unpacked source files (MockFileStore.store_source_package unpacks archives on storage, so we don't have the original bytes lying around). write/delete maintain a new _source_package slot. Lets execute-path tests against MockFileStore exercise the same code path as production now that _common_file_change_execute and start_preflight call these methods. [SUBMISSION-169] David
… store. The actual tar-building logic now lives on SubmissionFileStore as build_source_package, shared between this download controller and start_preflight's pre-call rebuild. Drops the local _build_source_tarball helper (47 lines moved into GsFileStore.build_source_package in the previous commit), simplifies imports, and updates the module docstring to spell out the new arrangement and why we still never read the persisted <id>.tar.gz directly. Behaviour unchanged for the download endpoint. [SUBMISSION-169] David
…on_file_change_execute now deletes file_store.delete_source_package(sid) alongside the existing preflight/user_decisions/directives/preview invalidations. Without this, a TeX upload's tar.gz would persist through a delete-all + PDF-upload sequence, and start_preflight would hand tex2pdf the old TeX-era archive (root cause of the 'Review Files shows stale files after PDF replacement' bug we've been chasing). Docstring rewritten to put source_package first, drop the old 'intentionally NOT deleted' claim, and explain why we delete here rather than rebuild here (rebuilding per event would be O(N^2) bucket traffic for multi-file uploads). [SUBMISSION-169] David
…eflight call. Replaces the long-standing TODO ('This tgz may be older than the files in the src dir, since submit 2.0 does not yet regenerate after file changes') with file_store.write_source_package(submission_id), the second half of the source-package staleness fix. Pairs with the deletion in _common_file_change_execute: file events delete the archive (cheap, one bucket call), preflight rebuilds it (one tar of N source files). tex2pdf is now guaranteed to scan the user's current source when /preflight is called, fixing the 'Review Files shows the old TeX file list after a delete-all + PDF upload' symptom we've been tracking. [SUBMISSION-169] David
DavidLFielding
requested review from
bdc34,
bmaltzan,
jweiskoff and
norbusan
as code owners
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.
I detected this bug when I drastically changed the set of files, replacing several TeX files with a single PDF, and ended up with the previous set of TeX files at the Review Files page. This was caused by our lack of updating the gzipped tar and subsequently feeding this to the preflight request. This PR ensured that the gzipped tar is refreshed when needed. It also removed the gzipped tar when the submitter starts making changes to the set of files. The code that creates the tar has been relocated.