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

task: Update go-api-client and stream-tester with fixes #43

Merged
merged 7 commits into from
Jun 14, 2022

Conversation

victorges
Copy link
Member

@victorges victorges commented Jun 14, 2022

This updates both stream-tester and go-api-client with the following fixes:

Upon fixing the usage of the libs, I also figured out that cancellation was not being
handled properly in the transcode and prepare tasks. Without livepeer/stream-tester#164
it actually wouldn't be fixable, since the segmenter didn't handle cancellation, but
now it does!

So this also finally fixes NOT #19 !!
Actually figured out that this only fixes cancellation on transcode/prepare. There may
still be something wrong with the import logic :(

Fixes error handlign on segmenter
If the loop finishes because ctx was cancelled, we'll
have no indication about the cancellation. Need to
check that explicitly.

Also notice that even if the segmenter sent the error on the
output channel, it would not be sufficient. That because if
the ctx is cancelled, it might end up never sending the error
anyway so we'd still need to check.
@victorges victorges requested a review from a team as a code owner June 14, 2022 15:11
Copy link
Collaborator

@figintern figintern left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@iameli iameli left a comment

Choose a reason for hiding this comment

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

LGTM

@victorges victorges merged commit 3783a26 into main Jun 14, 2022
@victorges victorges deleted the vg/chore/update-deps branch June 14, 2022 17:52
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.

3 participants