Skip to content

docs(docker): remove obsolete FIFTYONE_DATABASE_ADMIN=true install step#591

Merged
mgustineli merged 7 commits into
mainfrom
mgustineli/docker-readme-database-admin-cleanup
Jun 30, 2026
Merged

docs(docker): remove obsolete FIFTYONE_DATABASE_ADMIN=true install step#591
mgustineli merged 7 commits into
mainfrom
mgustineli/docker-readme-database-admin-cleanup

Conversation

@mgustineli

@mgustineli mgustineli commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Removes the obsolete install-time FIFTYONE_DATABASE_ADMIN: true guidance from the Docker deployment README and fixes the self-contradictory "Enable Database Admin mode" step, bringing Docker in line with Helm's existing v2.9+ treatment. The migration/upgrade safeguard section is left untouched.

Motivation

Per AS-578 and the cs-aloha thread, the "set FIFTYONE_DATABASE_ADMIN=true on first install" procedure is obsolete as of v2.9+. It became unnecessary in fiftyone core commit voxel51/fiftyone@44acde9 (June 2024): a fresh database now initializes to the connecting client's version, so there's nothing to migrate and the admin gate never fires on a fresh install. Because this is core fiftyone behavior shipped in the same images for both Helm and Docker, it applies to both — Helm's docs were already updated, but the Docker README lagged and even contradicted itself (Step 3 said true # Only for first install, while Step 4 showed false).

Changes (docker/README.md)

  • Step 3 — removed FIFTYONE_DATABASE_ADMIN: true # Only for first install from the compose.override.yaml example; fresh installs don't need it.
  • Step 4.1 — renamed "Enable Database Admin mode" → "Database admin mode" and rewrote the note. It previously showed false under an "enable" heading and claimed false "allows the application to create and migrate the database schema" (misleading). It now states admin mode isn't required for a fresh install (v2.9+), that false is the safe default, and that true is only for intentional migrations (links to Upgrades).
  • Upgrades section — unchanged; the migration safeguard (false to prevent auto-migration + the fiftyone migrate behavior) is still correct.

Test plan

  • Docs-only change; reviewed rendered diff.
  • No fresh-deployment test required: the obsolescence is in shared fiftyone core code (deployment-agnostic), confirmed via the commit history above (44acde9a53).

Refs: AS-578 · voxel51/fiftyone@44acde9a53

🤖 Generated with Claude Code

The "set FIFTYONE_DATABASE_ADMIN=true on first install" procedure is obsolete
as of v2.9+ (fiftyone core commit 44acde9a53, June 2024): a fresh database now
initializes to the connecting client's version, so the admin gate never fires
on a fresh install. Aligns Docker with Helm's existing v2.9 treatment; leaves
the migration/upgrade safeguard section intact.

Refs: AS-578

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mgustineli mgustineli requested a review from a team as a code owner June 30, 2026 14:26
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 419532cd-f199-45fd-bdac-db6e6fd18255

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

Updates docker/README.md to add an environment variable comment placeholder in the compose.override.yaml example and revises Step 4 database admin mode guidance: fresh installs (v2.9+) auto-initialize the database, so FIFTYONE_DATABASE_ADMIN defaults to false, with true reserved for intentional migrations.

Changes

Docker README deployment instructions

Layer / File(s) Summary
Compose override and DB admin guidance
docker/README.md
Adds a comment placeholder for env var overrides in the compose snippet; replaces old "Enable Database Admin mode" instructions with v2.9+ guidance that fresh installs don't require it, keeping FIFTYONE_DATABASE_ADMIN as false by default, and adds a cautionary note that true is only for intentional migrations.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐇 A README was tended with care,
New defaults now floating in air.
"Keep false by default!"
said the rabbit, "Exalt
migrations only when truly aware!" 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states the main docs change: removing obsolete FIFTYONE_DATABASE_ADMIN install guidance.
Description check ✅ Passed The description matches the README changes and explains the obsolete admin-mode guidance update.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mgustineli/docker-readme-database-admin-cleanup

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…ding

Fixes markdownlint MD051/link-fragments after renaming the Step 4 heading.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread docker/README.md
Comment thread docker/README.md Outdated
mgustineli and others added 3 commits June 30, 2026 16:51
Co-authored-by: Kevin DiMichel <56850465+kevin-dimichel@users.noreply.github.com>
Signed-off-by: Murilo Gustineli <52141042+mgustineli@users.noreply.github.com>
Co-authored-by: Kevin DiMichel <56850465+kevin-dimichel@users.noreply.github.com>
Signed-off-by: Murilo Gustineli <52141042+mgustineli@users.noreply.github.com>
Apply markdownlint-fix (collapse double blank line) and markdown-toc
(drop stale "Official Docker Images" TOC entry for the removed heading)
so the pre-commit CI check passes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@kevin-dimichel kevin-dimichel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

After #565 there are merge conflicts that need resolution.

Comment thread docker/README.md

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Image

There's a funky element here that needs to be fixed locally (not in the UI).

  • remove lines 213-217

Comment thread docker/README.md
…dme-database-admin-cleanup

# Conflicts:
#	docker/README.md
@mgustineli mgustineli merged commit a2330b1 into main Jun 30, 2026
11 checks passed
@mgustineli mgustineli deleted the mgustineli/docker-readme-database-admin-cleanup branch June 30, 2026 23:09
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