feat: Process uploaded submissions asynchronously#5580
Merged
rjsparks merged 34 commits intoietf-tools:mainfrom May 9, 2023
Merged
feat: Process uploaded submissions asynchronously#5580rjsparks merged 34 commits intoietf-tools:mainfrom
rjsparks merged 34 commits intoietf-tools:mainfrom
Conversation
* Rename process_uploaded_submission to process_and_accept_... * Remove outdated tests Does not yet test new behavior.
The PDF submission field was removed, so no need to test it.
Fixes remaining failing SubmitTest tests
Properly separating the upload and async processing stages of submission is a bigger refactoring than will fit right now. This better exercises the submission pipeline.
Codecov Report
@@ Coverage Diff @@
## main #5580 +/- ##
==========================================
- Coverage 88.80% 88.73% -0.07%
==========================================
Files 285 285
Lines 39757 39807 +50
==========================================
+ Hits 35307 35324 +17
- Misses 4450 4483 +33
|
NGPixel
approved these changes
May 8, 2023
rjsparks
requested changes
May 8, 2023
Member
rjsparks
left a comment
rjsparks
left a comment
There was a problem hiding this comment.
As discussed on a different channel - some of the text on the status page should be updated to call extra attention to the change of behavior.
| return self.clean_file("txt", PlainParser) | ||
| return self._clean_file("txt", PlainParser) | ||
|
|
||
| def clean_pdf(self): |
Member
Author
There was a problem hiding this comment.
In fact, that entire class (DeprecatedSubmissionManualUploadForm) is obsolete. Thank you for catching!
rjsparks
approved these changes
May 9, 2023
rjsparks
added a commit
that referenced
this pull request
May 11, 2023
* feat: Easy extraction of qualified volunteer list for nomcom chair (#5578) * feat: Easy extraction of qualified volunteer list for nomcom chair * fix: tune test setup to years where eligibility calculations can return nonempty * chore: revert unintended change * feat: default string when no affiliation is provided * chore: pin django-oidc-provider (#5588) * fix: add a link to the simplified volunteer view (#5583) * chore: add bibxml-ids dir to container build. (#5590) * chore: add bibxml-ids dir to container build. (#5590) * feat: Process uploaded submissions asynchronously (#5580) * fix: Use relative URL for submission status link * refactor: Refactor/rename process_uploaded_submission async task * feat: Add async task to process but not accept a submission * feat: Replace upload_submission() with an async implementation (WIP) * fix: Do not put Submission in "uploaded" state if an error occured * refactor: Improve text/XML draft processing flow * feat: Extract authors from text in async processing * fix: Fix call signatures and abort submission on failed validation * feat: Validate submission name format * fix: Correctly validate emails from text submission * fix: Clean up submission validation * fix: Better display errors on upload_submission page * feat: Reload submission status page when awaiting validation * test: Fix call signatures; remove unused imports * chore: Add type hint * test: Update tests to match renamed task * fix: Fix typo in error message * test: Fix failing Api- and AsyncSubmissionTests * Rename process_uploaded_submission to process_and_accept_... * Remove outdated tests Does not yet test new behavior. * refactor: Break up submission_file() helper * test: Refactor tests to run the async processing (wip) * test: Drop test of bad PDF submission The PDF submission field was removed, so no need to test it. * test: Update more tests * test: Bring back create_and_post_submission() and fix more tests * fix: Drop to manual, don't cancel, on revision inconsistency Fixes remaining failing SubmitTest tests * style: Restyle upload_submission() with black * test: Verify that async submission processing is invoked on upload * test: Bring back old do_submission and fix tests Properly separating the upload and async processing stages of submission is a bigger refactoring than will fit right now. This better exercises the submission pipeline. * fix: Accept only XML for API submissions * test: Test submission processing utilities * feat: Improve status display for "validating" submissions * chore: Remove obsolete code * test: Update test to match amended text --------- Co-authored-by: Robert Sparks <rjsparks@nostrum.com> * chore(deps): update all Yarn dependencies (#5564) * chore(deps): update all Yarn dependencies * chore: fix yarn cache --------- Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com> Co-authored-by: NGPixel <github@ngpixel.com> * chore(deps): update all npm dependencies for dev/deploy-to-container (#5587) Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com> * chore: Remove mysqlclient dependency (#5589) * fix: close open things (#5593) * fix: close open things * fix: clean up test created files * fix: remove one close too many * chore: add git safe directory to docker init script * chore: move git safe directory command to top * chore: remove debugging file write from test (#5598) --------- Co-authored-by: NGPixel <github@ngpixel.com> Co-authored-by: Jennifer Richards <jennifer@staff.ietf.org> Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
This replaces the validation/checking done in the
upload_submission()view with asynchronous processing. This uses a celery task in a similar way to theapi_submission()handling.Important This renames the
process_uploaded_submission()task toprocess_and_accept_uploaded_submission()and reuses the original name. I think these are the best names for the tasks, but it does introduce the possibility of mishandling tasks. To avoid this, the celery queue must be emptied before deploying the new code. This can be done by bringing the Django server down, waiting for any existing tasks to complete, then shutting down the celery containers and continuing with the deployment. It'd be best to deploy at a time when submissions are not frequent to minimize the chance that this requires down time.This drops support for uploading PDF files. Text file uploads are still accepted through the interactive
upload_submission()endpoint and processed as before. Theapi_submission()endpoint still only accepts XML.When a draft is in the "validating" state, the
submission_status()view now uses javascript to reload the page every 30 seconds. This can be disabled with a toggle switch and should be tolerant of javascript being disabled (but the page will not reload). It would be possible to make this more sophisticated, starting by using the API submission status endpoint to check whether a reload is needed, but that is left as a future exercise.