Make the database adapter available inside shouldExecute()#1100
Merged
Conversation
The adapter was only set on the migration/seed by the Environment, right before running the migration body - after the shouldExecute() gate in the Manager. As a result shouldExecute() could not query the database (calling getAdapter() threw "Adapter not set."), so eligibility could only depend on local application state, not the state of the database. Set the environment's adapter on the migration and seed before the shouldExecute() check. getEnvironment()->getAdapter() is memoized, so this adds no extra connection, and the Environment still (re)sets and wraps the adapter before executing the body, leaving migrate/rollback behavior unchanged.
markstory
approved these changes
Jul 9, 2026
markstory
left a comment
markstory
left a comment
Member
There was a problem hiding this comment.
Seems reasonable to me. We can't remove the existing setAdapter() calls as they are load-bearing because of Environment::executeMigration().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1099.
shouldExecute()could not use the database to decide eligibility. The adapteris only set on the migration/seed by
Environment::executeMigration()/executeSeed(), immediately before the migration body runs - which is afterthe
shouldExecute()gate inManager. So anygetAdapter()/execute()/query()/hasTable()call insideshouldExecute()threwRuntimeException: Adapter not set., limiting eligibility checks to localapplication state only.
Change
Manager::executeMigration()andManager::executeSeed()now attach theenvironment's adapter to the migration/seed before the
shouldExecute()check:Environment::getAdapter()memoizes its adapter, so this adds no extraconnection.
Environment::executeMigration()still sets and (fordown/change) wraps the adapter before running the body, so migrate/rollbackbehavior is unchanged -
shouldExecute()simply gains a read-capable adapter.This keeps the adapter lazy and Environment-owned, rather than moving
setAdapter()to migration construction time (which has no direction/environmentcontext and would fight the record-adapter wrapping used for reversible
migrations).
Tests
Added
testMigrationShouldExecuteCanQueryDatabase, backed by a newShouldExecuteWithDbfixture whoseshouldExecute()calls$this->getAdapter()->hasTable(...). It errors withAdapter not set.withoutthe fix and passes with it.
Docs
Documented that the adapter is available inside
shouldExecute(), with anexample.