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.

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

HTTP url output support #82

Merged

Conversation

CaitlinOCallaghan
Copy link
Contributor

Based off of Joey Parrish's HTTP Upload commit

Feature

  • Allow users to specify a HTTP url as an output
  • Segments and manifests will be uploaded to the specified url via HTTP PUT
  • Urls ending with a '/' will have the slash removed to prevent issues with Google Cloud

Testing

  • Added unit tests to check for config compatibility errors, such as specifying an HTTP url output with a "multi_period_input" and with "segment_per_file" set to false

Shortcomings

  • The multi period input list feature is currently structured to use the local file system and does not easily support HTTP uploads. This must be addressed as the multi period input feature continues to evolve, @meryacine.
  • The HTTP upload feature does not make sense if used with a cloud url, and although there is an error check for this, there is no unit test. The cloud url unit test would have required a mock bucket to pass the cloud url validity checks. Because of the work involved in writing a mock bucket and the future (and hopefully soon) removal of the CloudNode, it was decided that forgoing this particular unit test was okay.

tests/tests.js Outdated
it('fails when multiperiod_inputs_list is used with a HTTP url output', async () => {
const inputConfig = {
'multiperiod_inputs_list': [
getBasicInputConfig(),
Copy link
Member

Choose a reason for hiding this comment

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

style nit: indent +2 spaces here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed! :)

@joeyparrish
Copy link
Member

Doh! Sorry I overlooked your last changes on this. Merging now!

@joeyparrish joeyparrish merged commit 8cd8537 into shaka-project:master Aug 2, 2021
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants