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

feat(http): Add DELETE method support #1442

Merged
merged 2 commits into from
Oct 25, 2024
Merged

feat(http): Add DELETE method support #1442

merged 2 commits into from
Oct 25, 2024

Conversation

joeyparrish
Copy link
Member

When we try to remove an old segment from a live stream we uploaded via HTTP, we need to send DELETE requests.

@joeyparrish
feat(http): Add DELETE method support
b69b61c 
When we try to remove an old segment from a live stream we uploaded via HTTP, we need to send DELETE requests.
@joeyparrish joeyparrish requested a review from cosmin October 24, 2024 03:36
@joeyparrish
Copy link
Member Author

I'm still in the process of verifying this change. Converting to draft.

@joeyparrish joeyparrish marked this pull request as draft October 24, 2024 03:37
cosmin
cosmin approved these changes Oct 24, 2024
View reviewed changes
Copy link
Contributor

@cosmin cosmin left a comment

Choose a reason for hiding this comment

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

lgtm. should we add an option to enable/disable this behavior? If DELETE was never implemented then someone might be relying on the existing behavior, and it might be good to have an option to revert to this behavior at least temporarily.

@joeyparrish
Copy link
Member Author

I am not sure there's a storage API out there that doesn't have both PUT and DELETE. And without DELETE, your cloud storage fills up and you have to clean it out periodically with some other process. My guess is that HTTP output support was half-implemented and never seriously used. I think this PR finishes the feature for the first time.

@joeyparrish
Copy link
Member Author

I suppose preserved_segments_outside_live_window could be configured in a way that would never attempt to delete files. If anyone is using it that way, then this isn't going to impact them anyway.

@joeyparrish joeyparrish marked this pull request as ready for review October 24, 2024 04:49
@joeyparrish
Copy link
Member Author

Verified on my live stream!

I'm going to leave this open for comment for another day or so, in case anyone thinks I'm being unreasonable about not putting this change behind a flag.

@cosmin
Copy link
Contributor

cosmin commented Oct 24, 2024

I agree that preserved_segments_outside_live_window seems like a reasonable workaround for anyone that doesn't want to delete.

joeyparrish added a commit to joeyparrish/shaka-streamer that referenced this pull request Oct 24, 2024
@joeyparrish
feat(cloud): Add cloud delete support
a8e6baf 
This allows Packager to remove old segments from cloud storage automatically.

See shaka-project/shaka-packager#1442
@joeyparrish joeyparrish mentioned this pull request Oct 24, 2024
Merged
joeyparrish added a commit to shaka-project/shaka-streamer that referenced this pull request Oct 24, 2024
@joeyparrish
feat(cloud): Add cloud delete support (#164)
aa1f4e7 
This allows Packager to remove old segments from cloud storage
automatically.

See shaka-project/shaka-packager#1442
@joeyparrish
Copy link
Member Author

It's been nearly 24 hours without comment, so I'm going to merge this. If this ruins somebody's day, we can add a flag later. I suspect it will be fine.

@joeyparrish joeyparrish merged commit ddeacb2 into shaka-project:main Oct 25, 2024
36 checks passed
@joeyparrish joeyparrish deleted the http-delete branch October 25, 2024 03:13
@shaka-bot shaka-bot mentioned this pull request Oct 24, 2024
Merged
joeyparrish pushed a commit that referenced this pull request Oct 28, 2024
@shaka-bot
chore(main): release 3.3.0 (#1444)
cca6cc0 
🤖 I have created a release *beep* *boop*
---


##
[3.3.0](v3.2.1...v3.3.0)
(2024-10-25)


### Features

* Add IAMF support
([#1416](#1416))
([dc6196d](dc6196d)),
closes
[#1415](#1415)
* EXT-X-SESSION-KEY support
([#1427](#1427))
([d88ed27](d88ed27)),
closes [#36](#36)
* **http:** Add DELETE method support
([#1442](#1442))
([ddeacb2](ddeacb2))


### Bug Fixes

* **http:** Fix "Failed sending data to the peer" errors
([#1443](#1443))
([2c9d100](2c9d100))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cosmin cosmin cosmin 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.
2 participants
@joeyparrish @cosmin