Prevent sites from making too many API requests #11
Conversation
This may help prevent more requests on high traffic sites.
includes/Customer.php
Outdated
| $throttle = Transient::get( self::THROTTLE ); | ||
| $retry_count = (int) \get_option( self::RETRY_COUNT, 0 ); | ||
|
|
||
| if ( false !== $throttle || $retry_count >= 10 ) { |
There was a problem hiding this comment.
Shouldn't this be
| if ( false !== $throttle || $retry_count >= 10 ) { | |
| if ( true === $throttle || $retry_count >= 10 ) { |
There was a problem hiding this comment.
We want to return if the transient is not set, which will return explicit false. When it exists, the value (1) will be returned.
includes/Customer.php
Outdated
| @@ -123,6 +137,13 @@ public static function connect( $path ) { | |||
| return $provided; | |||
| } | |||
|
|
|||
| $throttle = Transient::get( self::THROTTLE ); | |||
There was a problem hiding this comment.
Do we need a throttle and a retry count? Can't we just use retry count?
There was a problem hiding this comment.
We're using the transient to track when to retry, and the retry count to prevent excessive retries.
includes/Customer.php
Outdated
| $timeout = HOURS_IN_SECONDS * $retry_count; | ||
| } | ||
|
|
||
| Transient::set( self::THROTTLE, 1, $timeout ); |
There was a problem hiding this comment.
This gets set, but never unset.
There was a problem hiding this comment.
The THROTTLE transient is basically a blocker that is meant to only self-expire. We shouldn't need to delete it.
There was a problem hiding this comment.
We shouldn't need to delete this. It can just expire which would allow the site to retry eventually.
This reverts commit a3fb0ba.
Proposed changes
This adds a throttle transient that will prevent sites from hitting the API too many times.
Type of Change
Checklist
Further comments