plugins.nbcnews: Plugin fix #4664
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bastimeyer
requested changes
Jul 18, 2022
Member
bastimeyer
left a comment
There was a problem hiding this comment.
Thanks for the PR. The rest is looking fine.
The plugin could use a rewrite/cleanup though, but that's not important right now... (parsing HTML and querying XPath instead of matching a regex, moving schemas away from class attribute definitions, replacing single-quote strings, using self.id instead of video_id, etc)
| from streamlink.plugin import Plugin, pluginmatcher | ||
| from streamlink.plugin.api import validate | ||
| from streamlink.stream.hls import HLSStream | ||
| from streamlink.stream import HLSStream |
Member
There was a problem hiding this comment.
Don't change the HLSStream import
| from streamlink.plugin.api import validate | ||
| from streamlink.stream.hls import HLSStream | ||
| from streamlink.stream import HLSStream | ||
| from streamlink.utils import parse_json |
Member
There was a problem hiding this comment.
Don't import this. The validate.parse_json() method should be used.
Comment on lines
+41
to
+44
| validate.get('cdnSources'), | ||
| validate.get('primary'), | ||
| validate.get(0), | ||
| validate.get('sourceUrl'), |
Member
There was a problem hiding this comment.
validate.get can receive a tuple of strings which will call __getitem__ on each successive object, which is much better than calling validate.get lots of times.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The stream.nbcnews.com API host used in the plugin recently stopped working. It fixes of plugin for the new API endpoint.