-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(video-api): add DELETE route for videos #412
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me!
b077776
Signed-off-by: Connor Doman <domanc@student.ubc.ca>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is just waiting on #393 right?
Hey @connordoman, would I bother you to bring the changes in #393 needed for this in a separate PR for review? |
Rebase pls :)) |
This PR/issue depends on: |
00eb775
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great, awesome work Connor!
Rebase and resolve conflicts pls. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a little more and rebase pls.
|
||
// delete the video from s3 | ||
try { | ||
deleteArtifact(videoRef, getOutputBucket()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following #560, the videoref in db can be under review or accepted. This means, in the case of videos being under review, deleting files in output bucket will leave files in input bucket stale/dangling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One way to go around this is to fetch the video tags from S3 first, check if it has privacypal-status
tag. If so, also try to delete the video from input bucket.
Co-authored-by: Thuan Vo <thuan.votann@gmail.com> Signed-off-by: Connor Doman <connor@connordoman.dev>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Doing a final test now...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
Welcome to PrivacyPal! 👋
Fixes: #411
Depends on #393
Depends on #403
Depends on #406
Depends on #388
Description of the change:
This change introduces an API route for deleting videos from S3 and the database.
Motivation for the change:
Clients and professionals need to be able to delete uploaded media as a matter of privacy.
Usage
DELETE /api/video/{awsRef}?appt={apptId}
e.g.
DELETE /api/video/1-privacy-pal-test-20240128T170000-processed.mp4?apptId=123