[BD-27] FEAT: Add new API endpoint for uploading transcripts - #27844
Conversation
|
Thanks for the pull request, @kaizoku! I've created BLENDED-876 to keep track of it in Jira. More details are on the BD-27 project page. When this pull request is ready, tag your edX technical lead. |
openedx-webhooks
added
open-source-contribution
alangsto
changed the title
openedx-webhooks
added
blended
3827262 to
d311154
Compare
kaizoku
changed the title
openedx-webhooks
added
needs triage
and removed
waiting on author
|
@edx/masters-devs-cosmonauts tagging for more visibility. This PR is needed for improvements to the video/transcript upload tool that is part of CC2OLX. Alan had to make similar changes to allow for video upload using API credentials in https://github.com/edx/edx-platform/pull/25513. |
schenedx
left a comment
schenedx
left a comment
There was a problem hiding this comment.
Requested to add logging statements. Rest looks straightforward.
| ) | ||
| response = JsonResponse(status=201) | ||
| except (TranscriptsGenerationException, UnicodeDecodeError): | ||
| response = JsonResponse( |
There was a problem hiding this comment.
I added a log message here. Does the language look alright?
| }, | ||
| file_data=ContentFile(sjson_subs), | ||
| ) | ||
| response = JsonResponse(status=201) |
There was a problem hiding this comment.
Could we please add logging statement for the success case? It really helps monitoring
There was a problem hiding this comment.
I added a message here as well, also including the edX video ID and transcript language.
|
jenkins run python |
1 similar comment
|
jenkins run python |
|
Thanks for the review @schenedx . Should I add some log messages to the existing input validation function as well? If not, and the log messages I've just added look good I can squash the extra commits here. |
|
Your PR has finished running tests. There were no failures. |
|
@kaizoku Thanks for adding those additional log messages. I don't think you need to add log messages to the existing input validation function, as errors that are caught in that function will ultimately be returned by the new endpoint you are adding. |
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
|
EdX Release Notice: This PR has been deployed to the production environment. |
…#27844) * FEAT: Add new API endpoint for uploading transcripts * Add log messages to transcript upload function
6 participants
Description
This adds another view and URL route for an endpoint which accepts transcript uploads. As opposed to the existing endpoints, this accepts JWT tokens as credentials. The existing endpoints only work for currently logged in users. This fills a need to programmatically upload transcripts from external scripts.
This doesn't alter existing behavior, but does extend and re-use code from the existing
transcript_upload_handler()view.Useful information to include:
Supporting information
This was needed for use on openedx/cc2olx#61. Initial discussion for the change began there.
Testing instructions
curl -d 'token_type=jwt&grant_type=client_credentials&client_id=xxxxxxxxxxxxxxxxxxx&client_secret=xxxxxxxxxxxxxx' http://localhost:18000/oauth2/access_tokencurl -H 'Authorizations: JWT xxxxxxxxxx' http://localhost:18010/upload_transcript_api/ -F file=@${HOME}/devstack/edx-platform/common/test/data/uploads/uk_transcripts.srt -F language_code=en -F edx_video_id=ae5f4ec8-7b27-447b-b094-080f74657e03Deadline
June 7th, 2021
Other information
Include anything else that will help reviewers and consumers understand the change.
No, but will be utilized in cc2olx.
The naming in the repurposing of
validate_transcript_upload_data()feels a bit awkward, but in the pursuit of DRY it seems better to re-use the form validation as these are very similar endpoints.cc: @alangsto