Merge pull request #6362 from voxel51/sid/add_enterprise_gs #6419
Conversation
Adding Docs for Getting Started with FOE
WalkthroughDocumentation updates across the Enterprise section: added a new Getting Started guide, expanded API connection configuration details, updated terminology to “FiftyOne Enterprise” and “FiftyOne Enterprise Python SDK,” and adjusted the Enterprise index to include a new callout and navigation entry. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
docs/source/enterprise/migrations.rst (2)
81-87: Tiny spacing cleanup (optional)Remove extra space before the link for polish.
-`Unlike FiftyOne Open Source <https://voxel51.com/docs/fiftyone/user_guide/config.html#database-migrations>`_, +`Unlike FiftyOne Open Source <https://voxel51.com/docs/fiftyone/user_guide/config.html#database-migrations>`_,
109-117: Tiny spacing cleanup (optional)Remove double space.
-Each version of FiftyOne Enterprise corresponds to a version of FiftyOne Open -Source called its "open source compatibility version", and this versioning +Each version of FiftyOne Enterprise corresponds to a version of FiftyOne Open +Source called its "open source compatibility version", and this versioning
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
docs/source/images/enterprise/getting_started_cloud_creds.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/enterprise/getting_started_import_samples.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/enterprise/getting_started_install_io_plugin.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/enterprise/getting_started_install_sdk.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/enterprise/getting_started_schedule_compute_metadata.gifis excluded by!**/*.gif,!**/*.gif
📒 Files selected for processing (7)
docs/source/enterprise/api_connection.rst(1 hunks)docs/source/enterprise/app.rst(1 hunks)docs/source/enterprise/getting_started.rst(1 hunks)docs/source/enterprise/index.rst(2 hunks)docs/source/enterprise/installation.rst(1 hunks)docs/source/enterprise/migrations.rst(8 hunks)docs/source/enterprise/roles_and_permissions.rst(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-12T22:15:20.635Z
Learnt from: swheaton
PR: voxel51/fiftyone#5096
File: docs/source/enterprise/roles_and_permissions.rst:0-0
Timestamp: 2024-11-12T22:15:20.635Z
Learning: In the `docs/source/enterprise/roles_and_permissions.rst` file, when documenting UI paths for the FiftyOne Enterprise App, the UI still uses "Team" in the navigation path (e.g., "Settings > Team > Users"), even after the rebranding from "Teams" to "Enterprise".
Applied to files:
docs/source/enterprise/roles_and_permissions.rst
🔇 Additional comments (12)
docs/source/enterprise/index.rst (2)
71-76: Getting Started callout and toctree entry look goodCallout item and navigation entry correctly reference getting_started.rst. No concerns.
170-170: Nav label addition LGTM“Getting Started <getting_started>” entry is consistent with the new page.
docs/source/enterprise/roles_and_permissions.rst (2)
41-44: Minor copyedit: “log in” and remove double spaceRecommend:
- “may log in” (verb) instead of “may login”
- Remove extra space before “deployment”
[ suggest_optional_refactor ]- Invited users may login using any identity provider that has been enabled on - your deployment. If you need more information about configuring IdPs or + Invited users may log in using any identity provider that has been enabled on + your deployment. If you need more information about configuring IdPs or
8-16: Branding/wording updates LGTMIntro, groups, and permissions sections read clearly and align with Enterprise terminology.
Also applies to: 105-108, 144-147
docs/source/enterprise/migrations.rst (1)
21-39: General changes LGTMTerminology and flows are consistent; commands/readme snippets look correct.
Also applies to: 47-52, 133-136, 142-177
docs/source/enterprise/installation.rst (1)
8-15: Intro and note LGTMClearer positioning and support contact updates look good.
Also applies to: 18-21
docs/source/enterprise/getting_started.rst (4)
156-201: SDK import examples LGTMPatterns for S3, GCS, Azure, and MinIO creation and persistence look correct.
Also applies to: 202-226, 227-251
256-275: Metadata guidance LGTMcompute_metadata usage and rationale are clear.
Also applies to: 276-282, 315-330
147-149: Verify cross-reference target:ref:
this page <enterprise-cloud-media>may not exist; consider linking to the “Cloud-backed media” doc label or page slug (cloud_media).
90-100: Verify anchors and method availability
- Ensure enterprise-plugins-install anchor exists.
- Confirm fom.test_api_connection() is available in the Enterprise SDK.
Also applies to: 287-296
docs/source/enterprise/app.rst (1)
129-134: Verify anchor for “cloud functions”The reference :ref:
cloud functions <enterprise-cloud-functions>may 404 if the anchor isn’t defined. Please verify.Run to confirm anchors exist:
docs/source/enterprise/api_connection.rst (1)
9-10: API config table and wording LGTM; verify option names existDocs look correct. Please confirm the config keys and env vars exist in code (api_uri, api_key, api_compressor, api_transfer_method) to avoid drift.
Also applies to: 30-45
| .. code-block:: python | ||
|
|
||
| brew install ffmpeg | ||
|
|
There was a problem hiding this comment.
Fix code block language for macOS FFmpeg
Use shell, not python.
- .. code-block:: python
+ .. code-block:: shell📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .. code-block:: python | |
| brew install ffmpeg | |
| .. code-block:: shell | |
| brew install ffmpeg |
🤖 Prompt for AI Agents
In docs/source/enterprise/getting_started.rst around lines 49 to 52 the code
block is marked as python but contains a shell command for macOS; change the
code-block directive from python to shell (or sh/bash) so syntax highlighting
and semantics match the brew install ffmpeg command.
| export MINIO_ACCESS_KEY_ID=... | ||
| export MINIO_SECRET_ACCESS_KEY=... | ||
| export MINIO_DEFAULT_REGION=... | ||
|
|
There was a problem hiding this comment.
MinIO env var names inconsistent with Installation page
Use the same variables and include the endpoint URL to work with MinIO.
- export MINIO_ACCESS_KEY_ID=...
- export MINIO_SECRET_ACCESS_KEY=...
- export MINIO_DEFAULT_REGION=...
+ export MINIO_ACCESS_KEY=...
+ export MINIO_SECRET_ACCESS_KEY=...
+ export MINIO_ENDPOINT_URL=...
+ export MINIO_REGION=... # if applicable📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export MINIO_ACCESS_KEY_ID=... | |
| export MINIO_SECRET_ACCESS_KEY=... | |
| export MINIO_DEFAULT_REGION=... | |
| export MINIO_ACCESS_KEY=... | |
| export MINIO_SECRET_ACCESS_KEY=... | |
| export MINIO_ENDPOINT_URL=... | |
| export MINIO_REGION=... # if applicable |
🤖 Prompt for AI Agents
In docs/source/enterprise/getting_started.rst around lines 143 to 146, the MinIO
environment variable names are inconsistent with the Installation page and lack
the endpoint URL; update these lines to use the same variable names used in
Installation (e.g. MINIO_ROOT_USER and MINIO_ROOT_PASSWORD) and add an export
for the MinIO endpoint (e.g. MINIO_ENDPOINT_URL=https://minio.example.com), so
the docs align and include the endpoint required to connect.
| To create a new dataset, click on the "New dataset" button in the upper right | ||
| corner of the FiftyOne Enterprise homepage. A pop-up will appear alowing you to | ||
| choose a name and optional description/tags for the dataset: | ||
|
|
There was a problem hiding this comment.
Typo: “alowing” → “allowing”
- corner of the FiftyOne Enterprise homepage. A pop-up will appear alowing you to
+ corner of the FiftyOne Enterprise homepage. A pop-up will appear allowing you to📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| To create a new dataset, click on the "New dataset" button in the upper right | |
| corner of the FiftyOne Enterprise homepage. A pop-up will appear alowing you to | |
| choose a name and optional description/tags for the dataset: | |
| To create a new dataset, click on the "New dataset" button in the upper right | |
| corner of the FiftyOne Enterprise homepage. A pop-up will appear allowing you to | |
| choose a name and optional description/tags for the dataset: |
🤖 Prompt for AI Agents
In docs/source/enterprise/getting_started.rst around lines 300 to 303, fix the
typo "alowing" to "allowing" in the sentence describing the dataset creation
pop-up; update the text to read "...A pop-up will appear allowing you to choose
a name and optional description/tags for the dataset:".
2 participants
Moving PR #6362 (Enterprise Getting Started docs update by @sherpan ) into production so the changes can be deployed as soon as possible.
Summary by CodeRabbit