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

plugins.twitch: fix call_subdomain #2958

Merged
merged 3 commits into from
May 13, 2020
Merged

plugins.twitch: fix call_subdomain #2958

merged 3 commits into from
May 13, 2020

Conversation

EnterGin
Copy link
Contributor

@EnterGin EnterGin commented May 9, 2020

fixes #2751

@codecov
Copy link

codecov bot commented May 9, 2020

Codecov Report

Merging #2958 into master will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #2958      +/-   ##
==========================================
- Coverage   52.61%   52.59%   -0.03%     
==========================================
  Files         251      251              
  Lines       15756    15762       +6     
==========================================
  Hits         8290     8290              
- Misses       7466     7472       +6     

Copy link
Member

@bastimeyer bastimeyer left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time finding the issue.
Ideally the whole call_subdomain method should get removed, as the API hostname could also be set as a parameter in the regular call method, which would be way more clear, but that's not important now. A simple try/finally block is enough.

Comment on lines 309 to 315
try:
response = self.call(path, format=format, schema=schema, **extra_params)
except PluginError:
self.subdomain = subdomain_buffer
raise
self.subdomain = subdomain_buffer
return response
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
try:
response = self.call(path, format=format, schema=schema, **extra_params)
except PluginError:
self.subdomain = subdomain_buffer
raise
self.subdomain = subdomain_buffer
return response
try:
return self.call(path, format=format, schema=schema, **extra_params)
finally:
self.subdomain = subdomain_buffer

@bastimeyer bastimeyer added the plugin issue A Plugin does not work correctly label May 12, 2020
@EnterGin EnterGin requested a review from bastimeyer May 12, 2020 19:44
Copy link
Member

@bastimeyer bastimeyer left a comment

Choose a reason for hiding this comment

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

That's not the diff I suggested. You are writing duplicate code.

@EnterGin
Copy link
Contributor Author

EnterGin commented May 12, 2020

That's not the diff I suggested. You are writing duplicate code.

Oh, you about removing call_subdomain at all?

@bastimeyer
Copy link
Member

diff

#2958 (comment)

@EnterGin
Copy link
Contributor Author

OK, I got it, sorry.

@EnterGin EnterGin requested a review from bastimeyer May 12, 2020 21:26
@EnterGin EnterGin closed this May 13, 2020
@EnterGin EnterGin reopened this May 13, 2020
@bastimeyer bastimeyer changed the title plugins.twitch: --retry-streams stop working fix plugins.twitch: fix call_subdomain May 13, 2020
@bastimeyer bastimeyer merged commit 76880e4 into streamlink:master May 13, 2020
Billy2011 pushed a commit to Billy2011/streamlink-27 that referenced this pull request May 14, 2020
Billy2011 pushed a commit to Billy2011/streamlink-27 that referenced this pull request May 26, 2020
back-to pushed a commit to back-to/streamlink that referenced this pull request Jul 10, 2020
mkbloke pushed a commit to mkbloke/streamlink that referenced this pull request Aug 18, 2020
resiproxy pushed a commit to resiproxy/streamlink that referenced this pull request Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin issue A Plugin does not work correctly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Twitch --retry-streams stop working after internet connection loss and recovery
2 participants