Skip to content

FIX Check all classes and don't rely on connection_attempted()#11807

Merged
emteknetnz merged 1 commit intosilverstripe:6from
creative-commoners:pulls/6/fix-database-checks
Jul 29, 2025
Merged

FIX Check all classes and don't rely on connection_attempted()#11807
emteknetnz merged 1 commit intosilverstripe:6from
creative-commoners:pulls/6/fix-database-checks

Conversation

@GuySartorelli
Copy link
Copy Markdown
Member

@GuySartorelli GuySartorelli commented Jul 28, 2025

DB::is_active() will lazy-connect to the database if we haven't done so already. That means the DB::connection_attempted() check was actually preventing us from connecting to the database even though we were ready to do so.

There's no need to add that check after checking DB::is_active() because it'll either be active (and we're good to go) or not (and we should say we're not ready).

Issues


// Don't check again if we already know the db is ready for this class.
if (!empty($this->tableReadyClasses[$class])) {
if (!empty($this->tableReadyClasses[$required])) {
Copy link
Copy Markdown
Member Author

@GuySartorelli GuySartorelli Jul 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops! This was only ever checking the same class over and over instead of the one in the loop 😅
The unit tests were incorrect so it looked like things were working as expected.

@GuySartorelli GuySartorelli marked this pull request as draft July 28, 2025 23:48
@GuySartorelli GuySartorelli marked this pull request as ready for review July 28, 2025 23:51
DB::is_active() will lazy-connect to the database if we haven't done so
already. That means the DB::connection_attempted() check was actually
preventing us from connecting to the database even though we were ready
to do so.

There's no need to add that check after checking is_active() because
it'll either be active (and we're good to go) or not (and we should say
we're not ready).
@GuySartorelli GuySartorelli force-pushed the pulls/6/fix-database-checks branch from d199c9c to f502863 Compare July 29, 2025 00:16
@emteknetnz emteknetnz merged commit e8d42f9 into silverstripe:6 Jul 29, 2025
12 checks passed
@emteknetnz emteknetnz deleted the pulls/6/fix-database-checks branch July 29, 2025 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants