-
Notifications
You must be signed in to change notification settings - Fork 1
feat: Requeue DO environment var documentation [FOEPD-2878] #496
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughDocumentation and test coverage added for two new environment variables controlling operation requeueing behavior: Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @docker/README.md:
- Line 831: Fix the typo in the README table for the environment variable
FIFTYONE_DO_REQUEUE_DELAY_MINUTES: change the description text from "requeud" to
"requeued" so the sentence reads "The number of minutes an operation can sit in
queued before being requeued. Default is 30 minutes."
In @helm/fiftyone-teams-app/README.md:
- Line 690: Update the README table entry for
apiSettings.env.FIFTYONE_DO_REQUEUE_DELAY_MINUTES to correct the typo: change
"requeud" to "requeued" so the description reads "The number of minutes an
operation can sit in queued before being requeued."
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
helm/fiftyone-teams-app/values.schema.jsonis excluded by!**/*.jsonhelm/fiftyone-teams-app/values.yamlis excluded by!**/*.yaml
📒 Files selected for processing (3)
docker/README.mdhelm/fiftyone-teams-app/README.mdtests/unit/helm/api-deployment_test.go
🧰 Additional context used
🪛 LanguageTool
docker/README.md
[grammar] ~831-~831: Ensure spelling is correct
Context: ...peration can sit in queued before being requeud. Default is 30 minutes. ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: helm-integration / integration-helm
- GitHub Check: helm-unit / unit-helm
- GitHub Check: pre-commit
- GitHub Check: docker-pulls
🔇 Additional comments (1)
tests/unit/helm/api-deployment_test.go (1)
497-504: LGTM! Test coverage is comprehensive.The new environment variables are correctly added to all test scenarios with consistent default values ("30" and "3"), ensuring proper validation across different configurations.
Also applies to: 603-610, 723-730, 840-847, 945-952, 1050-1057
|
@CamronStaley - the other test indentation issues are fixed in #492 |
Co-authored-by: Kevin DiMichel <[email protected]> Signed-off-by: Camron Staley <[email protected]>
swheaton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚢 🇮🇹
Rationale
https://voxel51.atlassian.net/browse/FOEPD-2878
This voxel-hub pr introduces the concept of "requeueing" stuck operations so I am documenting the new environment vars here to reflect those: https://github.com/voxel51/voxel-hub/pull/2017
Changes
Add two new env vars for requeuing stuck DOs
Checklist
Testing
Automated tests + testing on that other PR
Notes
This feature will be released in the next release. Once the release branch is made, we will update the target branch.