-
Notifications
You must be signed in to change notification settings - Fork 8
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
Youtube service #77
Youtube service #77
Conversation
if ($videoList === null){ | ||
$videoList = base64_encode(serialize($this->client->getPlaylistItemsByPlaylistId($this->playlist))); | ||
$this->cache->set($cacheKey, $videoList); | ||
$this->cache->expireat($cacheKey, strtotime('+5 hours')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the cache can be set to something a bit further away in time. It only happens once a month at most that we post a video, so it doesn't have to be on the website immediately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, so 24 hours or would you prefer going with a few days?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
24 hours would be better, yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also: make it a class constant for easier tweaking and more context.
Ok while trying to get this sorted Google is frustrating me by refusing to accept my API key. Not sure what is going on but research in hacking together a small client myself to try and find the issue. This is annoying me for over a month. |
amsterdamphp_site.integration.youtube_client: | ||
class: "%amsterdamphp_site.integration.youtube_client.class%" | ||
factory_service: amsterdamphp_site.integration.youtube_factory | ||
factory_method: getClient |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok i'm fine with this, even if you could drop the factory and just intantiate the client directly as a service, but this gives us a place to set defaults if we ever need them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After talking with you this was my assumption as how you preferred to have it setup. (Heck I took you meetup bundle as example.) If this is fine I'll let it be but if you really prefer instantiating the client directly I'll change it to that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine like this 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I/m fine with this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! 👍
Also, how on earth did you get a YouTube API key? 😖 And what playlist value did you use? |
You get the API key from: https://console.developers.google.com |
Checked it out and it is a great service! 💖 |
Saturday deploy? :D |
@dennisdegreef not really, now someone has to build a front onto this 😉 . |
well.. if the server goes belly up, we need support from True, since we are at true.. i guess we are safe :P |
true |
yes, True |
|
|
Youtube service for #72