Support transcribing a url #107
Conversation
…ed a job from sqs, uses yt-dlp to download and uploads the result to s3
Co-authored by : Marjan Kalanaki <marjan.kalanaki@guardian.co.uk>
…ion to get the secret
philmcmahon
changed the title
| @@ -0,0 +1,17 @@ | |||
| FROM python:3.12-bookworm | |||
| WORKDIR /opt | |||
| LABEL com.theguardian.transcription-service.media-download-container="Media download container with yt-dlp, associated dependnencies and media download app" | |||
There was a problem hiding this comment.
| LABEL com.theguardian.transcription-service.media-download-container="Media download container with yt-dlp, associated dependnencies and media download app" | |
| LABEL com.theguardian.transcription-service.media-download-container="Media download container with yt-dlp, associated dependencies and media download app" |
| '-o', | ||
| 'StrictHostKeyChecking=no', | ||
| '-D', | ||
| '1337', |
There was a problem hiding this comment.
might be worth putting this port in a variable?
| @@ -11,21 +11,31 @@ export const runSpawnCommand = ( | |||
| cmd: string, | |||
| args: ReadonlyArray<string>, | |||
| logStdout: boolean, | |||
| logImmediately: boolean = false, | |||
There was a problem hiding this comment.
Might be worth enforcing at a type level that we never log immediately if processName == transcribe. If its a transcription of sensitive source material we don't want the transcript in the logs. We could make processName a union of string literals.
| {Object.entries(uploads).map(([key, value]) => ( | ||
| <li className="flex items-center"> | ||
| <span className={'mr-1'}>{iconForStatus(value)}</span> | ||
| {key} {value === RequestStatus.Invalid && ' (invalid url)'} |
There was a problem hiding this comment.
it would be good to add a button to reset the UI in this case
| mediaSource, | ||
| }: { | ||
| reset: () => void; | ||
| mediaSource: 'file' | 'url'; |
There was a problem hiding this comment.
could define this as a type since it's in a few places
| const urlsWithStatus = Object.fromEntries( | ||
| urls.map((url) => [ | ||
| url, | ||
| checkUrlValid(url) ? RequestStatus.InProgress : RequestStatus.Invalid, |
There was a problem hiding this comment.
This feels more like form validation than async error reporting.
Is it tricky to get the form to point out invalid urls before its submitted?
| "extends": "@guardian/tsconfig/tsconfig.json", | ||
| "compilerOptions": { | ||
| "module": "CommonJS", | ||
| "noUnusedLocals": false |
There was a problem hiding this comment.
do we want unused locals?
| client: s3Client, | ||
| params: { | ||
| Bucket: bucket, | ||
| Key: `downloaded-media/${metadata.title}.${metadata.extension}`, |
There was a problem hiding this comment.
Seems possible that two different files will share a title. Could this result in files being accidentally overwritten or permissions leaking to the wrong user?
There was a problem hiding this comment.
I might be missing something but I'm having a hard time working out what the difference is between the media-downloader and media-download packages. Should they be separate or did you mean to rename media-download? It looks like there's some overlap in functionality e.g. implementations of uploadToS3 and execution of yt-dlp.
What does this change?
Following from #104, this PR provides the user interface and infrastructure needed to support transcription of media urls submitted via the UI. It also includes an update to the media-download service to support running with a proxy server.
Depends on https://github.com/guardian/investigations-platform/pull/523
Infrastructure
The media-download service itself is built into a docker container - you can see the changes to the CI github actions workflow to see how this is built. Currently that container is published only to ECR (AWS).
After a user submits a url, a request is made to the transcription API. The API puts a message on a (new) media download SQS task queue, then returns a success code back to the client.
We use an event bridge pipe to connect the SQS queue to the media-download step function. The pipe does the job of reading the message, passing it as an input property to the step function, and then deleting the message. Pipes can do data validation/manipulation, but at the moment it's just a dumb pipe - read, trigger, delete. If an invalid message is sent then it is up to the media-download service to deal with it.
The media-download service itself is an ECS task wrapped in a step function. The main change other than the proxy service is that the service no longer needs to deal with the incoming SQS queue - it just gets the contents of the message as an environment variable (thanks to the event bridge pipe).
In order to add an ECS Task to the transcription service without having to check in the CDK context file, I had to make some changes to GuCdk which are not yet merged - this PR is currently against my branch on the CDK project. The associated PRs are here:
Separately, there is a proposal to remove the ecs task patterns from gucdk since no other teams are using it, but for now it's staying there
Discussion here on how to ship logs from this task guardian/cloudwatch-logs-management#360
Other changes
The output handler has been updated to provide a download url for the media. In a future piece of work we'll add export to google drive.
Finally, the client side UploadForm has had a small makeover, mainly with the second page you get to after submitting the form being pulled out into smaller component files. This change is probably best tested by trying out the forms locally or on CODE.
How to test
Currently live on CODE https://transcribe.code.dev-gutools.co.uk/
How can we measure success?
The intention here is for journalists to be able to keep a record of media they are using for journalistic purposes