Conversation
When the direct engine is requested (the default) and the existing state
still uses terraform, convert the state to the direct engine before the
run proceeds, instead of after deploy. deploy/destroy commit the migration
(resources.json written and pushed, terraform.tfstate backed up); plan and
summary keep it in memory. The migration runs after phases.Build so its
conversion and plan check see library and ${artifacts.*} references
resolved (matching what a direct deploy records), and it falls back to the
terraform engine if the plan check fails. Terraform-state cleanup after the
commit is best-effort: resources.json already outranks the terraform state
by serial, so a failed backup/delete only warns.
Co-authored-by: Isaac <no-reply@databricks.com>
Now that the migration runs before deploy, a hard error blocks a deploy that would otherwise succeed on terraform. Treat every failure before resources.json is pushed (parse, conversion, empty-state sweep) as non-fatal: warn and deploy on terraform this time, retrying the migration next run - matching what plan-check and push failures already did. Only a failure after the push (placing/opening the local state) stays an error, since the workspace is already committed to direct; reword those messages to say the migration succeeded and re-running recovers. Co-authored-by: Isaac <no-reply@databricks.com>
The pre-deploy migration plan-checks the converted state. If that plan would recreate (destroy + create) an existing resource, do not commit the migration: a recreate risks data loss, whether it comes from a conversion that did not faithfully reproduce an immutable field or from a real pending config change (which terraform would recreate too). Fall back to terraform this run and retry the migration next run, once the recreate is applied. Records direct_migrate_recreate_planned. checkPlanOnTempState now returns the plan so the caller can inspect the planned actions. Co-authored-by: Isaac <no-reply@databricks.com>
The pre-deploy migration flips the engine terraform->direct, but the SDK user agent only appends dimensions, so tagging engine/terraform in PullResourcesState and then engine/direct after a successful migration left both on a migrated deploy's requests. PullResourcesState now skips the tag on the auto-migration path (direct requested, state still terraform); the caller sets engine from the resolved stateDesc.Engine once the migration has run, been skipped, or fallen back - so a migrated deploy is engine/direct, a fallback is engine/terraform, and there is never a stale second tag. The two generate commands, which never migrate, tag the resolved engine too. auto-migrate-envvar now records the deploy's engine tags to lock this in. Co-authored-by: Isaac <no-reply@databricks.com>
Records (no fix) how the pre-deploy migration treats a schema's named id fields when they change or are backend-normalized: - name backend-lowercased (config MySchema vs deployed myschema): spurious warnOnIDFieldRename warning (also emitted by the terraform deploy's dry-run telemetry), but the follow-up plan converges. - catalog_name legitimately changed (immutable id): only a warning, no recreate - the migration records the config value and the moved-catalog drift is silently stable. - storage_root trailing slash normalized: absorbed by normalize_slash, no warning, converges. Co-authored-by: Isaac <no-reply@databricks.com>
…odels, pipelines Extends the schema characterization (no fix) across more resources: - volume name backend-lowercased: spurious rename warning, converges (same as schema name). - volume_type changed (both a provided id field AND recreate_on_changes): the recreate classification wins - the recreate guard fires and the run stays on terraform (contrast with catalog_name, a pure provided id field, which only warns). - registered_model catalog_name changed (immutable id): warn + silently stable drift, no recreate (same as schema catalog_name). - pipeline storage changed (pure recreate_on_changes): recreate guard fires. Catalogs are direct-only (no terraform converter), so they are never a migration scenario and are intentionally not covered. Co-authored-by: Isaac <no-reply@databricks.com>
The migration seeded id-composing fields (provided_id_fields, updatable_id_fields) from config, which snapshotted a pending id change as already applied: a genuinely-moved/renamed resource then plan-converged and silently drifted from the backend, while a backend-normalized value (identifier case, trailing slash) produced a spurious "rename not applied" warning - also leaked onto plain terraform deploys via the dry-run telemetry. reconcileIDFields now seeds each id field from the deployed terraform value unless it differs from config only by backend normalization (case-insensitive + trailing-slash), in which case the config value is kept. So a real id change surfaces in the plan (recreate for provided_id -> caught by the recreate guard; rename update for updatable_id), and a normalized value converges with no warning. warnOnIDFieldRename is removed (its genuine case is now the recreate guard, its false-positive case is gone). Regenerated the id-field characterization goldens, which now assert the fixed behavior (recreate on genuine change, clean converge on normalization). Co-authored-by: Isaac <no-reply@databricks.com>
The terraform fallback leaves two terraform.tfstate* files, and find lists them in filesystem order, which differs on Windows. Pipe the finds through sort so the golden is deterministic across platforms. Co-authored-by: Isaac <no-reply@databricks.com>
First cloud coverage for the pre-deploy migration: renaming a schema (an immutable provided id field) must recreate. Verifies against a real workspace that the recreate guard detects the recreate, falls back to terraform (which recreates the schema under the new name), and the following deploy migrates the now-matching state to direct. Passed on aws-cli. Co-authored-by: Isaac <no-reply@databricks.com>
Companion to cloud-recreate-schema. A volume name is an updatable id field, so unlike a schema (which recreates) it is renamed in place. Verifies against a real workspace that the migration seeds the deployed name, the plan renames the volume (UpdateWithID) rather than snapshotting the new name as applied, the migration succeeds (no fallback), and the schema alongside it is untouched. Passed on aws-cli. Co-authored-by: Isaac <no-reply@databricks.com>
Extend the two schema recreate tests (local auto-migrate-recreate and cloud cloud-recreate-schema) with a step that deploys the recreate WITHOUT --auto-approve first: the migration guard falls back to terraform, which refuses the destructive recreate (exit 1) and changes nothing. The following --auto-approve step then applies it. The cloud step passed on aws-cli. Co-authored-by: Isaac <no-reply@databricks.com>
Third cloud resource type. A registered model name is a provided id field, so a rename recreates - the migration guard detects it against the real backend and falls back to terraform, which recreates the model; the retry then migrates. Unlike schemas/volumes, registered models are not in the destructive-approval group, so the recreate applies without --auto-approve. Passed on aws-cli. Co-authored-by: Isaac <no-reply@databricks.com>
…nario> Drop the redundant cloud-/idfield-/recreate-/auto-migrate- prefixes: migration and running on both testserver and cloud are the defaults, and Cloud=true only adds a cloud run. Name by resource and scenario instead (schema-rename, volume-type-change, ...). Remove auto-migrate-rename, now fully covered by the more thorough schema-rename (which also runs on cloud). Co-authored-by: Isaac <no-reply@databricks.com>
…lume) Upgrade schema-name-normalized and volume-name-normalized to Cloud=true (unique names + cleanup trap). This covers the last behavior class on cloud: a backend-normalized id (UC lowercasing) converges through the migration with no warning or recreate. Verified on aws-cli. Together with schema-rename, volume-rename and registered-model-rename, all id-field/recreate classes now run against a real workspace. Co-authored-by: Isaac <no-reply@databricks.com>
…alog catalog_name is an immutable provided id field; moving a schema to another catalog recreates it. Create the destination catalog out of band (catalogs are direct-only, so they cannot be a bundle resource here), then verify on a real workspace: the recreate is refused without --auto-approve and, with it, moves the schema to the new catalog; the retry migrates. Passed on aws-cli. Co-authored-by: Isaac <no-reply@databricks.com>
…f-band catalog Moving a registered model to another catalog recreates it (catalog_name is a provided id field). Create the destination catalog out of band (it auto-creates a default schema the moved model reuses). Registered models are not in the destructive-approval group, so the recreate applies without --auto-approve; the retry migrates. Passed on aws-cli. Co-authored-by: Isaac <no-reply@databricks.com>
storage is a recreate_on_changes field on pipelines. Give the pipeline a notebook library and a DBFS storage path (no external location needed) so it deploys on a real workspace, then verify: changing storage plans a recreate; pipelines are in the destructive-approval group, so it is refused without --auto-approve and applied with it; the retry migrates. Passed on aws-cli. Co-authored-by: Isaac <no-reply@databricks.com>
…only schema-storage-normalized, schema-storage-change and volume-type-change need a registered external location on a real workspace, so they are not Cloud-enabled. Add a note in each pointing to the cloud-enabled tests that cover the same class. Co-authored-by: Isaac <no-reply@databricks.com>
migrate/command holds the explicit "bundle deployment migrate" command tests; migrate/auto holds the auto-migration-on-deploy tests. Drop the now-redundant auto-migrate- prefix from the latter. The shared script.prepare and test.toml stay at migrate/ and are inherited by both subtrees. Co-authored-by: Isaac <no-reply@databricks.com>
…esource A recreate in the migrated state's first plan is a genuine pending config change (the same one terraform would apply), and the deploy's approval flow already gates it behind --auto-approve. So stop refusing the migration on a planned recreate: commit the migrated state and let the normal approval gate the recreate on direct. direct_migrate_recreate_planned is kept purely as an observability metric. The migrate/auto recreate tests are updated to the new flow (migrate commits, then the deploy refuses/applies the recreate on direct). Co-authored-by: Isaac <no-reply@databricks.com>
…ne-storage-change On a real workspace a direct-engine pipeline recreate leaves a field the next plan re-updates (1 changed), while the testserver converges (1 unchanged); a single golden can't match both. The migrate + recreate-gating flow (the test's subject) is unaffected and still covered on cloud; the clean-redeploy path is covered by the schema/registered-model recreate tests. Co-authored-by: Isaac <no-reply@databricks.com>
…ng id compare reconcileIDFields is a no-op for permissions/grants sub-nodes (their adapters declare no id fields), so the len(parts)==3 guard was redundant - call it unconditionally. Every id-composing field is a string, so assert both values are strings (erroring otherwise) and inline the case- and trailing-slash-insensitive compare, dropping the speculative non-string structdiff fallback and the idFieldNormalizedEqual helper. Co-authored-by: Isaac <no-reply@databricks.com>
…migrate test The metric fires whenever a committing migration's plan would recreate a resource, but only schema-storage-change asserted it. Add print_recreate_planned_metric (a stable, single-key telemetry assertion) at the recreate step of the remaining recreate tests. The cloud tests (schema/pipeline/registered-model) need RecordRequests=true to read telemetry; assert only the recreate-planned key so backend-varying metrics (e.g. conversion warnings) don't destabilize the golden. Co-authored-by: Isaac <no-reply@databricks.com>
Use print_migration_telemetry (all direct_* metrics) instead of the recreate-only helper, matching schema-storage-change. At the recreate step the set is direct_migrate_recreate_planned + direct_migrated_via_env, both CLI-side and stable across engines/clouds. Drops the print_recreate_planned_metric helper. Co-authored-by: Isaac <no-reply@databricks.com>
When an id-composing field differs between config and the deployed terraform state (beyond backend normalization), reconcileIDFields now emits a warning naming the resource, field, and both values, and whether the resource will be recreated (provided-id) or renamed (updatable-id) - so the plan that follows the migration is not a surprise. Only fires on a genuine pending change; the dry-run telemetry path runs post-deploy where config already matches the state, so it stays quiet. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
A deploy that auto-migrates terraform→direct now prepares the converted state (plan-check, recreate metric, local state file) but does not commit it to the workspace until the deploy is approved: deployCore pushes it and FinalizeDeferredMigration then backs up the terraform state. A declined deploy discards the local state (DiscardDeferredMigration) and stays on terraform, and prints "Migration not committed, staying on terraform state". Destroy keeps committing the migration up front (MigrateCommit) since it has no deployCore to defer to; plan/run migrate in memory only (MigratePlan). This also moves the commit under the deploy lock. Because the state push now happens after deployCore applies changes, a push failure is a hard deploy error (self-healing on retry) rather than the old graceful terraform fallback; push-failure is rewritten accordingly. Co-authored-by: Isaac <no-reply@databricks.com>
…t path Destroy now uses the same deferred migration as deploy (MigrateDeferred): it writes the local direct state, and once the destroy is approved destroyCore runs on it and the terraform state is backed up (CleanupTerraformStateAfterMigration). A declined destroy discards the migration and stays on terraform. Backing up the local terraform.tfstate matters here: destroy removes the local resources.json, so a lingering terraform state would otherwise win the next deploy. With both deploy and destroy deferring, the early-commit path is unused, so MigrateCommit and its pushMigrationToRemote/finalizeLocalMigration/pushDirectState helpers are removed; the remaining modes are MigratePlan (plan, run) and MigrateDeferred (deploy, destroy). Co-authored-by: Isaac <no-reply@databricks.com>
Three fixes from an adversarial review of the deferred-migration flow: - Discard a prepared migration on any non-commit exit, not just a decline. A deploy/destroy that prepared the migration then failed before applying (a missing --plan file, a validation error) left the bundle silently on the local direct state. ProcessBundleRet now discards it in a deferred cleanup, and the deploy and destroy phases clear b.MigrationDeferred at approval so a committed run is kept and a post-approval failure retries on direct. - Unify the empty-terraform-state path into the deferred flow. The special up-front "sweep" branch ran outside approval and left a header-only WAL that persisted no state file; the converter now writes an empty base state, so empty states migrate like any other and are discarded if not committed. - Retire the superseded local terraform state inside destroyCore, before removing the direct state, so a crash between the two can never leave a live terraform.tfstate with no direct state for the next deploy to pick up. Co-authored-by: Isaac <no-reply@databricks.com>
The previous commit removed the local terraform state on every direct destroy, which broke a non-migrating direct destroy that runs alongside a separate, still-valid terraform.tfstate meant for a later deploy to migrate (it deleted that state, so the next deploy could not migrate and reset the serial). Gate the removal on the migration flag, passed into destroyCore, so it fires only for a destroy that migrated from terraform - which is the crash-window this closes. Co-authored-by: Isaac <no-reply@databricks.com>
…ard tests
plan-missing and empty-plan-missing assert on the "reading plan file" error,
whose text differs on Windows ("The system cannot find the file specified."
vs "no such file or directory"). Add a Repls entry that normalizes it, like
bundle/root/env-not-found, so the tests keep running on Windows and cover the
discard file operations there rather than being skipped.
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
The DATABRICKS_BUNDLE_ENGINE=terraform deploy in state_present actually runs on the direct engine because the higher-serial direct state wins engine resolution. That was only implied by the section title (the engine/direct assertion was commented out), and a nearby comment misattributed the lingering terraform state to that deploy rather than to the initial terraform deploy pulled from the remote. Restore the User-Agent assertion after the deploy so the direct-run is verified, and correct the comment. Test-only. Co-authored-by: Isaac <no-reply@databricks.com>
…approval Replace the MigrateMode enum and the CommitStateMigration ProcessOptions flag with two composable worker functions the caller sequences (process.go), so every command follows the same path regardless of intent: - Migrate converts the terraform state to direct and opens it in memory - reversible, nothing on disk or in the workspace. plan, run, and a declined deploy just drop it and stay on terraform. - CommitMigration is the point of no return: it persists the converted state (serial tf+1), pushes it, retires the terraform state (local rename + remote backup), and records the source. Deploy calls CommitMigration right after the plan is approved and before it applies anything, so the migration commits at tf+1 and the deploy then advances the state to tf+2. Committing at approval keeps the model decisive - once approved, the bundle is on direct - at the cost of a window where a deploy that then fails has still migrated. Destroy commits as part of its teardown (files.Delete owns the remote state; no source telemetry). Because Migrate writes nothing durable, the discard machinery is gone: b.MigrationDeferred, DiscardDeferredMigration, dstate.DiscardWrite, and ProcessBundleRet's discard-defer are removed - there is never an on-disk uncommitted migration to clean up. Messaging is now truthful: the "migrating to direct" notice prints once the conversion succeeds (for every migration, not just the default engine, and without "automatically"); "Migrated N resources" prints only at the commit, so a refused or failed deploy no longer claims a migration that did not happen. Co-authored-by: Isaac <no-reply@databricks.com>
The rebase onto main left the migration goldens in main's (migrate-after-deploy) form; regenerate them against this branch's migrate-before-deploy code so they match Migrate/CommitMigration output. Co-authored-by: Isaac <no-reply@databricks.com>
… fell back
b.MigratingToDirect is set as intent (direct requested, state was terraform) before Migrate
runs. When Migrate's plan check fails it falls back to terraform - migrated=false, the state
is left on terraform and never opened - but b.MigratingToDirect stays true. Gating deploy's
CommitMigration (and destroy's teardown commit, and the decline warnings) on b.MigratingToDirect
alone therefore ran CommitMigration on a fallback, whose Persist() then failed on the unopened
state ("failed to save resources state to \"\""). Gate on the resolved engine actually being
direct (stateEngine.IsDirect()) so a fallback proceeds on terraform gracefully.
Caught by Windows CI (the empty-path rename error text is OS-specific); the bug was present on
all platforms but masked by a regenerated golden on Linux/macOS.
Co-authored-by: Isaac <no-reply@databricks.com>
Cleanup from a code review of the Migrate/CommitMigration split: - Migrate never used its requiredEngine argument (only CommitMigration needs it); drop it from Migrate and from migrateTerraformToDirect and their callers. - convertTFStateToDirect returned a resourceCount both callers discarded (CommitMigration counts via StateDB.ExportState instead); drop it. - Fix stale comments: the converter no longer renames a temp file into place (the caller reads it into memory), and correct the serial description - a populated conversion lands at tf+2 (its own WAL replay bumps the tf+1 base once more), an empty one stays at tf+1; either way it outranks the terraform state. No behavior change. Co-authored-by: Isaac <no-reply@databricks.com>
…e delete The 403 was injected at OFFSET=0, which hits the artifacts/.internal cleanup delete, not the terraform.tfstate delete - so the tf delete succeeded and the test never exercised the retirement-failure path (the remote state dir showed only terraform.tfstate.backup). The tf delete is the second workspace/delete of a migrating deploy, so fault it at OFFSET=1. The golden now shows the "could not delete terraform.tfstate" warning, the migration succeeding anyway (best-effort), and both the remote terraform.tfstate and its .backup remaining. Co-authored-by: Isaac <no-reply@databricks.com>
…t-approval window) CommitMigration is the point of no return: it runs after approval and before the apply, so a deploy that fails during the apply has still durably migrated. This test forces that window - a job change makes the migrating deploy attempt a jobs/reset, which is faulted (403) after the migration commits - and asserts the bundle is on direct (resources.json present, terraform state retired, direct_migrated_via_env recorded) even though the deploy errored, and that a retry finishes the apply on the direct state without re-migrating. Co-authored-by: Isaac <no-reply@databricks.com>
…s WAL A migrating destroy ran on the in-memory converted state and called UpgradeToWrite without first persisting a base (unlike deploy's CommitMigration). If the destroy was interrupted between UpgradeToWrite and Finalize, it left an orphan .wal with no state file - the next run re-migrated in memory and tripped UpgradeToWrite's O_EXCL on that orphan WAL, wedging the bundle until the WAL was removed by hand. Persist the base first: an interrupted destroy now leaves a committed direct state on disk, so the next run resolves to it (a direct state) and its file-backed Open recovers the WAL instead of re-migrating. destroyCore still removes the local state at the end, so a successful destroy is unchanged. Co-authored-by: Isaac <no-reply@databricks.com>
…tion, no commit) bundle run needs resource ids, so on a terraform state with direct requested it migrates in memory to resolve them - but it is read-only w.r.t. the engine. This test asserts the run fires after an in-memory migration yet leaves nothing durable: no resources.json, no resources.migrating temp, no WAL, terraform.tfstate intact, and no migration-source telemetry. The bundle stays on terraform; a later deploy is what commits. Co-authored-by: Isaac <no-reply@databricks.com>
Migration produces a plain direct state with no deployment_history feature, so deploying with DATABRICKS_BUNDLE_DEPLOYMENT_HISTORY=true against a terraform state is a mismatch. This asserts it is rejected after the in-memory migration but before CommitMigration writes or pushes anything - the bundle stays on terraform with no leaked direct state. Migration is otherwise non-DMS only, so this combination had no coverage. Co-authored-by: Isaac <no-reply@databricks.com>
…ed-after-prep) The migration is prepared in memory (in process.go) before the destroy phase asks for approval, so a refused destroy is the prepared-but-not-committed path. Strengthen the refusal assertions to confirm nothing durable is left behind - no resources.json, no resources.migrating temp, no WAL - only the terraform state remains. (An interactive "no" hits the same drop but is not acceptance-testable, since there is no TTY to prompt.) Co-authored-by: Isaac <no-reply@databricks.com>
A migration committed by deploy retires the terraform state by renaming the local file to terraform.tfstate.backup. A later destroy ran on the direct engine but removed only the direct state, so the .backup lingered after the deployment was gone. Remove it on any direct destroy - it is never read as live state, so this is always safe - which also lets the empty terraform/ dir be pruned. New acceptance test destroy-after-deploy covers deploy-migrate then destroy and asserts no state is left behind, local or remote. Co-authored-by: Isaac <no-reply@databricks.com>
Replace the per-name terraform.tfstate/resources.json/WAL finds in the destroy and run-no-commit tests with a single find.py listing (sorted, forward slashes, cross-platform, excludes the .terraform provider cache) plus a workspace list of the remote state directory. The goldens now show exactly which state is present locally and remotely, so a stray tfstate/.backup would surface. Co-authored-by: Isaac <no-reply@databricks.com>
…t migrate test The standalone destroy-after-deploy test re-deployed the same terraform->direct migration the default test already sets up. Append the destroy plus state-file listing (local find.py + remote workspace list) to default instead, and drop the duplicate test. Still asserts a destroy after a deploy-committed migration leaves no terraform.tfstate.backup, direct state, or remote deployment behind. Co-authored-by: Isaac <no-reply@databricks.com>
Remove the Terraform deployment engine: the tfdyn config writers, the plan/apply/init/install/import/unbind/showplanfile paths, and the terraform branches in the deploy, destroy, and bind/unbind phases. Only the direct engine deploys now. Pinning the removed engine via bundle.engine or DATABRICKS_BUNDLE_ENGINE now errors and points at CLI v1.18.x. An existing terraform state is still parsed and migrated to the direct engine before deploy/destroy (bundle/migrate and the terraform-state parser in util.go/pkg.go stay); a migration that cannot be completed is now a hard error rather than a fall-back to terraform. Drop the now-unused terraform-exec/hc-install dependencies, the terraform CI cell, and the terraform value from the bundle.engine schema enum. Co-authored-by: Isaac <no-reply@databricks.com>
The terraform engine is gone, so the acceptance suite runs on the direct engine only: drop the DATABRICKS_BUNDLE_ENGINE matrix and EnvVaryOutput from the shared test.toml files, and update the validate_engine unit test to expect an error (not a deprecation warning) for engine: terraform. Golden regeneration and the fixture-based migration tests follow in subsequent commits. Co-authored-by: Isaac <no-reply@databricks.com>
Collaborator
Integration test reportCommit: 91d67da
20 interesting tests: 20 FAIL
|
denik
added a commit
that referenced
this pull request
Sep 30, 2026
Remove the terraform engine implementation (apply/convert/plan/init/install/import/ interpolate/pubkey/showplanfile/unbind/write) and the entire tfdyn package; keep the state-mapping helpers the migration still needs by extracting them into statemap.go (convertPermissions/GrantsResourceNameToKey, BindOptions), matching the kept set in the reference removal PR #6864. Callers of the deleted engine functions are fixed in follow-up commits. Co-authored-by: Isaac <no-reply@databricks.com>
Contributor
Author
|
Superseded by the stacked terraform-removal PRs, which replace this monolithic one:
Closing in favor of those. |
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
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.
Removes the Terraform deployment engine. Only the direct engine deploys now.
engine: terraform(orDATABRICKS_BUNDLE_ENGINE=terraform) errors and points at CLI v1.18.x.terraformvalue from thebundle.engineschema enum.Stacked on #6749 (migrate-before-deploy).
Status / remaining work (draft):
This pull request and its description were written by Isaac.