eleweek / WatchPeopleCode

http://WatchPeopleCode.com
MIT License
198 stars 16 forks source link

Added total viewers to the API output - Untested #13

Closed Chr12t0pher closed 9 years ago

Chr12t0pher commented 9 years ago

I've added the code implemented in the sidebar bot into the main bot. It is currently untested so its probably a good idea to check it before using it in production however I don't see why it wouldn't work though.

Future options could be to store the output to the database, with the option to see how many people watched a past broadcast when it was live perhaps?

gkbrk commented 9 years ago

Hello!

Does YouTube return a viewer count at "https://www.youtube.com/live_stats?v=" after the stream is done and recorded?

What happens when you call "https://www.youtube.com/live_stats?v=" on a finished stream, there isn't a YT stream on WatchPeopleCode so I can't test.

eleweek commented 9 years ago

Hi! I am currently using different api to get viewers: https://github.com/WatchPeopleCode/WatchPeopleCode/commit/dced39e1bb8aa4e3ea90ba92b8dd5bd4c84d6e60 https://github.com/WatchPeopleCode/WatchPeopleCode/commit/cb03c45136f6683f9f41b505965819e169e98a3c (which seems to work) Since we make api calls in background process, the code in pull-request needs to be refactored

paked commented 9 years ago

Just out of curiosity, is there any particular reason as to why this isn't tested?

eleweek commented 9 years ago

You mean the first commit(dced39e)? IIRC: I did a test run but got distracted and forgot to check the results, so I assumed everything is ok.

paked commented 9 years ago

Was more looking at 775115a9a348ff6305bc6e3e549744a04e298cf1, Chris says it isn't tested... If it's because it's difficult to setup an environment this should be seen as a problem in itself...

eleweek commented 9 years ago

It is a pretty much standard heroku application, and all necessary environment variables are mentioned in README. I don't think it is difficult to set up if you're familiar with heroku.

paked commented 9 years ago

Well, to be fair it does have a lot of dependencies... and some of them you can't even fulfill (A bot with moderator access to the WPC subreddit isn't something everyone can have), but I guess this is a separate issue

eleweek commented 9 years ago

Well, to be fair it does have a lot of dependencies...

Library dependencies can be easily resolved using pip install -r requirements.txt

A bot with moderator access to the WPC subreddit isn't something everyone can have

Not true, you don't need moderator access to get submissions, and if you don't specify bot's login and password it will silently ignore flair-related code(as it should).

paked commented 9 years ago

Sorry, I meant "dependencies" in the "external services" way. I see your point though, but if these all fail gracefully why are they required vars?

eleweek commented 9 years ago

Well, if empty environment variables are confusing, we just simply stop requiring them. It won't be difficult to fix.

eleweek commented 9 years ago

Ok, I'm closing it, I manually merged these changes over a few commits. (We don't want to go to external apis from views)

eleweek commented 9 years ago

Thanks for contributing!