Skip to content
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.

Sign up for GitHub

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

Jump to bottom

After the derivative object is destroyed queue a job… #4064

Merged
merged 3 commits into from
Mar 26, 2020
Merged

After the derivative object is destroyed queue a job… #4064

merged 3 commits into from
Mar 26, 2020

Conversation

cjcolvar
Copy link
Member

…which deletes the actual file from the streaming server. This is based on the masterfile management delete job.

I'll add tests if this looks reasonable.

@phuongdh phuongdh force-pushed the derivative_delete branch from 3208544 to 2c8cf77 Compare March 25, 2020 18:47
@phuongdh phuongdh marked this pull request as ready for review March 25, 2020 19:09
cjcolvar
cjcolvar commented Mar 26, 2020
View reviewed changes
Copy link
Member Author

@cjcolvar cjcolvar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @phuongdh for adding the tests! Code-wise this looks good to me now. Did you test it out in docker?

@phuongdh
Copy link
Member

phuongdh commented Mar 26, 2020 •
edited
Loading

Thanks @phuongdh for adding the tests! Code-wise this looks good to me now. Did you test it out in docker?

Using minio, absolute_location protocol is http instead of s3 so it's not working.

Attempting to delete derivative http://minio:9000/derivatives/be3c7ad7-a615-4386-981d-a66784a67d86/outputs/sample-medium.mp4
Derivative http://minio:9000/derivatives/be3c7ad7-a615-4386-981d-a66784a67d86/outputs/sample-medium.mp4 does not exist

We could either make sure absolute_location looks like s3://derivatives/be3c7ad7-a615-4386-981d-a66784a67d86/outputs/sample-medium.mp4 (preferably) or do more URL wrangling in FileLocator.

@phuongdh phuongdh force-pushed the derivative_delete branch from 6382ac9 to 67746ae Compare March 26, 2020 15:57
@cjcolvar
Copy link
Member Author

If that change works then 👍.

@phuongdh phuongdh force-pushed the derivative_delete branch from 67746ae to 6b10cdc Compare March 26, 2020 18:12
@phuongdh
Copy link
Member

Confirmed working in development stack

Dananji
Dananji approved these changes Mar 26, 2020
View reviewed changes
Copy link
Contributor

@Dananji Dananji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@cjcolvar cjcolvar merged commit 06f2b30 into develop Mar 26, 2020
@cjcolvar cjcolvar deleted the derivative_delete branch March 26, 2020 18:53
@joncameron joncameron mentioned this pull request Jun 9, 2020
Closed
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dananji Dananji Dananji approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cjcolvar @phuongdh @Dananji