[SITE-2632] Update check_client_dependencies to check for Relay#471
[SITE-2632] Update check_client_dependencies to check for Relay#471earthlingdavey wants to merge 3 commits intopantheon-systems:mainfrom
Conversation
This PR fixes 2 cases where `WP_REDIS_USE_RELAY=true` - The class is not checked for existence. - The drop-in will not run if Redis extension is not installed. The fix: - Relay\Relay is correctly checked for existence. - The drop-in still works if the Redis extension is not installed.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
jazzsequence
left a comment
jazzsequence
left a comment
There was a problem hiding this comment.
if WP_REDIS_USE_RELAY constant does not exist, PHP errors are thrown. More logic needed to handle if that constant exists before using it in the condition.
object-cache.php
Outdated
| */ | ||
| public function check_client_dependencies() { | ||
| if ( ! class_exists( 'Redis' ) ) { | ||
| if ( ! class_exists( WP_REDIS_USE_RELAY ? 'Relay\Relay' : 'Redis' ) ) { |
There was a problem hiding this comment.
| if ( ! class_exists( WP_REDIS_USE_RELAY ? 'Relay\Relay' : 'Redis' ) ) { | |
| $has_relay = defined( 'WP_REDIS_USE_RELAY' ) && class_exists( 'Relay\Relay' ); | |
| if ( ! $has_relay || ! class_exists( 'Redis' ) ) { |
There was a problem hiding this comment.
Ah,I applied this change, but in fact we need to check defined('WP_REDIS_USE_RELAY') && WP_REDIS_USE_RELAY && class_exists( 'Relay\Relay' ), in the event the constant is set and false.
|
@pwtyler My suggested change should handle the condition that's failing the unit tests. Once merged, I think this is probably g2g. Needs changelog update and version bump if we're pushing a release. |
|
Hi @jazzsequence , thanks for catching this and for the suggestion. I can pick it up next week. |
pwtyler
changed the title
Co-authored-by: Chris Reynolds <[email protected]>
|
Hey folks, apologies for not thoroughly testing my first PR. Having read all feedback, would the following be ok?
|
|
@earthlingdavey at a glance, that looks like it would work. @pwtyler do you agree? |
3 participants
This PR fixes 2 cases where
WP_REDIS_USE_RELAY=trueThe fix: