Implemented youtube command#586
Conversation
|
@ChrisLovering it seems like it isn't possible for an error to occur on the users end: https://developers.google.com/youtube/v3/docs/errors#search, so I'll only implement the catchall. |
Shivansh-007
left a comment
Shivansh-007
left a comment
There was a problem hiding this comment.
Like said before (in Dev contribute) it would be good if this command acts like Reddit one, showing the viewcount and upvote.
|
Personally, I'm opposed to displaying the like/dislike ratio and view count because it seems like unnecessary info, but I'm totally up for your ideas. I think it would be helpful to show how useful it is, but I'm not sure if it is worth the space. |
|
Let’s give it a try and see how it looks. |
|
Alright, I'll have the time to implement it in an hour or so. |
Xithrius
added
area: backend
|
There is a typo in the word |
SuperMaZingCoder
added
the
status: needs review
|
|
||
| log = logging.getLogger(__name__) | ||
|
|
||
| KEY = Tokens.youtube |
There was a problem hiding this comment.
@ks129 specifically you meant the token wrapper, correct?
|
|
||
| async def format_search_result(self, index: int, result: List[Video]) -> str: | ||
| """Formats search result to put in embed.""" | ||
| return RESULT.format( |
There was a problem hiding this comment.
Maybe this is just my personal bias, but I think it's hard to read formatting code if the formatting template is in a different castle. Maybe put it nearby, or even inline it (as an f-string?)
There was a problem hiding this comment.
Does this include the SEARCH_API, STATS_API, YOUTUBE_VIDEO_URL, and YOUTUBE_SEARCH_URL constants too? Or are those fine as is considering that their names are self-explanatory compared to the RESULT f-string.
| STATS_API, | ||
| params={"part": "statistics", "id": id, "key": KEY}, | ||
| ) as response: | ||
| if response.status != 200: |
There was a problem hiding this comment.
As discussed in discord, maybe such errors should be left to a top-level handler
There was a problem hiding this comment.
@decorator-factory As it's been a bit since I've worked on this, were we thinking about making the request, and if it fails, just tell the user "Something went wrong!"
| """Sends the top 5 results of a query from YouTube with fifteen second cool down per user.""" | ||
| results = await self.search_youtube(search) | ||
|
|
||
| if results: |
There was a problem hiding this comment.
- I would rewrite it as
if not results:
... # sad path
return
# happy pathbut it's more of a taste question, I guess
There was a problem hiding this comment.
- If you don't ever differentiate between
[]orNone, why doessearch_youtubereturnOptional[List[...]]? Maybe you should show different errors in each case, or return just[]or justNonein case of error
Xithrius
added
status: waiting for author
|
@SuperMaZingCoder Still planning on finishing this off? |
|
@Xithrius I'm going to finish this off today. |
|
@SuperMaZingCoder what's the status on this? If you can't work on this anymore for any reason, I can pass it on to another person if needed. If you're planning on continuing this in the future, take your time. |
|
@SuperMaZingCoder are you still interested in doing this feature? If not, that's completely fine, happens to a lot of people including myself. As always, take the time that you need. |
|
I will be closing this PR due to inactivity, I know you'd like to finish this feature yourself but no progress has been made. Due to this project being restructured, the target file needs to be moved from You can reopen this when you'd like to continue this feature. |
8 participants
Relevant Issues
Closes #585
Description
Implemented a new
youtubecog which has ayoutube(alias:yt) command to search youtube from a specified query. Additionally, there is a fifteen-second cooldown.Reasoning
This allows users to share videos with each other and search for tutorials relevant to a conversation. I imagine staff might find this useful for linking tutorials that they recommend to other users.
Screenshots
Did you:
pipenv lock?pipenv run lint)?