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

Fix #1702 - ensure replacements are rerequested on failure #1703

Merged
merged 1 commit into from
Dec 13, 2016

Conversation

davemevans
Copy link
Contributor

As discussed in #1702, the fast switch changes meant that alternate base urls were not failed to in the event of a download failure. This is fixed by kicking the scheduler once the replacement queue has been populated.

Additionally, extra checks are needed to ensure that if it was an initialisation segment that failed this can be rerequested since next fragment rule will not replace these.

@davemevans
Copy link
Contributor Author

#1701 is also needed in also to make this work properly.

Copy link
Contributor

@dsparacio dsparacio left a comment

Choose a reason for hiding this comment

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

Review and all seem reasonable changes in scheduler.

@dsparacio
Copy link
Contributor

@bbcrddave thanks for this last minute change. Sorry If I broke this! Interesting enough, I remember us talking about this last May when the changes were put in and I was being very careful to check replacement for BASEURL but I guess I missed something. Was all BaseURL broken or just a certain use case?

@davemevans
Copy link
Contributor Author

All broken, but not a problem since obviously no one has been using it up to now 😉
Seems we need to improve our test coverage ...

@dsparacio dsparacio merged commit 627971b into Dash-Industry-Forum:development Dec 13, 2016
@davemevans davemevans deleted the 1702 branch December 13, 2016 21:16
@davemevans davemevans added this to the v2.4.0 milestone Dec 13, 2016
@dsparacio
Copy link
Contributor

OK thanks. Agreed. I am tired of talking about it - time for action. Time to backfill the missing tests, add proper coverage reporting, and start to only accept PR with test associated with the change. .

I will add it to the agenda

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants