fix: give each course import its own working directory - #39109
AhtishamShahid wants to merge 2 commits into
Conversation
Every import of a given course used a single working directory derived from the course key alone, shared by the import task and by both upload views. Two imports of the same course therefore overwrote each other's archive, and whichever import finished first deleted the other's extracted OLX mid-run. The surviving task failed with a bare filesystem error: "[Errno 116] Stale file handle" on an NFS-backed GITHUB_REPO_ROOT, "[Errno 2]" on a local disk. Add course_import_working_dir() and remove_course_import_working_dir(), and use them from import_olx and from both upload views so that every import and every upload gets a private subdirectory. Both upload views now drop their staging copy once the archive is in storage; previously they relied on the import task's cleanup of the shared directory. Cleanup no longer raises, so a failing cleanup can no longer mask the error that triggered it.
|
Thanks for the pull request, @AhtishamShahid! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Description
Two people importing the same course at the same time would break each other: every import of a course used one shared working directory, so whichever import finished first deleted the other's archive and extracted files mid-run. The second import then failed with a bare
[Errno 116] Stale file handle([Errno 2]on a local disk) that told the author nothing. Each import and each upload now gets its own private subdirectory, so both imports succeed instead of one failing. Affects Course Authors on any site where two people can import the same course; no config, migration, or API change.Supporting information
Reported on courses.learn, where two authors imported the same course two minutes apart. The originating ticket (
mitodl/hq#13190) is private, so the log is repeated here:Testing instructions
GITHUB_REPO_ROOTshould have no leftover directory for that course.pytest cms/djangoapps/contentstore/views/tests/test_import_export.py::ConcurrentImportTestCase, which fails on the parent commit with the production error and passes here.Already run locally on Python 3.12 in an
openedx-dev:22.0.0container: the four affected suites give 149 passed and 2 failed, where those 2 fail identically on the unpatched files (skew between that image's baked tree and master) andruff checkis clean; CI has not run yet.Other information
This does not make two concurrent imports of different archives into one course safe — they both rewrite the same course structure, so the result is whichever finishes last; refusing the second import up front would close that too, but it changes product behavior and is deliberately left out here.