-
Notifications
You must be signed in to change notification settings - Fork 21
Adds support for Akamai purging #51
base: master
Are you sure you want to change the base?
Conversation
Thanks @Hyra |
@ostark Anything left to do to get this merged in? ❤️ |
@Hyra A review was missing to get this merged, I guess ... |
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.
🙈 well, please have a look
@@ -257,14 +256,13 @@ protected static function handleUpdateEvent(Event $event) | |||
Plugin::getInstance()->trigger(Plugin::EVENT_BEFORE_PURGE, $purgeEvent); | |||
|
|||
// Push to queue | |||
\Craft::$app->getQueue()->push(new PurgeCacheJob([ | |||
\Craft::$app->getQueue()->ttr(1200)->push(new PurgeCacheJob( |
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.
Any good reason for ttr(1200)
?
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.
In our experience, with sites that have a lot of URLs the purge by url loop takes too long and times out with the default TTR. We actually purge by tag now to avoid this problem, so I guess the ttr might indeed be overkill in most cases.
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.
@Hyra Let's not hardcode this. Either make it a config option, or just remove it and users can control it with EVENT_BEFORE_PUSH
.
public function purgeAll() | ||
{ | ||
// TODO: Purge all in Akamai | ||
return true; |
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.
is there no endpoint?
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.
Unfortunately, no. Only by tag, cp code or url. CP Code could work as a purge all, but only if you set it up specifically.
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 had a quick look at the docs. cp code should work.
|
||
$context = stream_context_create($context); | ||
|
||
try { |
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.
there is nothing wrong with file_get_contents()
, but to be consistent I'd prefer GuzzleHttp\Client
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.
Fair enough, I'll change it ! 👍🏻
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.
@Hyra can you add return types to the methods?
@@ -257,14 +256,13 @@ protected static function handleUpdateEvent(Event $event) | |||
Plugin::getInstance()->trigger(Plugin::EVENT_BEFORE_PURGE, $purgeEvent); | |||
|
|||
// Push to queue | |||
\Craft::$app->getQueue()->push(new PurgeCacheJob([ | |||
\Craft::$app->getQueue()->ttr(1200)->push(new PurgeCacheJob( |
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.
@Hyra Let's not hardcode this. Either make it a config option, or just remove it and users can control it with EVENT_BEFORE_PUSH
.
$_ENV['AKAMAI_CLIENT_SECRET'] = getenv('AKAMAI_CLIENT_SECRET'); | ||
$_ENV['AKAMAI_ACCESS_TOKEN'] = getenv('AKAMAI_ACCESS_TOKEN'); | ||
$_ENV['AKAMAI_MAX_SIZE'] = getenv('AKAMAI_MAX_SIZE'); | ||
|
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.
These values should be coming from config, not getenv
, right?
Rather than writing a custom module I figured Akamai fits well within Upper's drivers.
Let me know what you think.