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

[Streams] Make preview picture for stream alerts bigger #3689

Merged
merged 1 commit into from Mar 25, 2020

Conversation

@Dav-Git
Copy link
Contributor

Dav-Git commented Mar 25, 2020

Type

  • Bugfix
  • Enhancement
  • New feature

Description of the changes

Closes #3685

@Dav-Git Dav-Git requested review from palmtree5 and Twentysix26 as code owners Mar 25, 2020
@Drapersniper

This comment has been minimized.

Copy link
Contributor

Drapersniper commented Mar 25, 2020

Hmm.. there’s a reason default was used here, iirc not every video will return the medium arg so this change can leave it susceptible to a crash

@jack1142

This comment has been minimized.

Copy link
Member

jack1142 commented Mar 25, 2020

@Drapersniper hmm, according to https://developers.google.com/youtube/v3/docs/videos#snippet.thumbnails.(key) it should be available for all videos (unlike standard and maxres) but if you are sure that's not true we can always use it like:

thumbnails.get("medium") or thumbnails.get("default")
@jack1142 jack1142 self-assigned this Mar 25, 2020
@jack1142 jack1142 added this to the 3.3.3 milestone Mar 25, 2020
Copy link
Member

jack1142 left a comment

Looks good to me.

// According to YouTube Data API Reference, default, medium and high thumbnails are always available and other sources also confirm that so there's no reason to believe that it can be unavailable for any video. But if this will actually raise KeyError for some video in future, you can blame me for believing in YouTube's docs too much 😄

@jack1142 jack1142 changed the title [Streams]Make preview picture for streamalerts bigger [Streams] Make preview picture for stream alerts bigger Mar 25, 2020
@jack1142 jack1142 merged commit 8c612a9 into Cog-Creators:V3/develop Mar 25, 2020
5 checks passed
5 checks passed
Lint Python
Details
Tox - Tests
Details
Tox - Style
Details
Tox - Docs
Details
Tox - Postgres (3.8)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.