test: unset flex_start_max_run_duration_minutes when null or zero#403
test: unset flex_start_max_run_duration_minutes when null or zero#403olivermeyer merged 1 commit intomainfrom
Conversation
olivermeyer
requested a review
from helmut-hoffer-von-ankershoffen
as a code owner
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
PR Review: test: unset flex_start_max_run_duration_minutes when null or zero🎯 SummaryStatus: ✅ APPROVED - Ready to merge This PR implements a temporary workaround for handling ✅ Critical Checks (All Passed)1. Test Markers ✅
2. Code Quality ✅
3. Conventional Commits ✅
4. Test Coverage ✅
🔍 Code Review DetailsChanges AnalysisFile: What Changed:
Why This Works:
Architecture Compliance ✅Modulith Principles: N/A (test file only)
Medical Device & Security ✅
Constants ReferenceFrom
Both environments have this value set to 💡 Suggestions (Non-Blocking)1. Enhanced TODO CommentCurrent (line 303): # TODO(oliverm): remove this conditional when applications handle null flex_start_max_run_duration_minutesSuggested: # TODO(oliverm): remove this conditional when applications handle null/zero flex_start_max_run_duration_minutes
# Expected fix: ETA end of January 2026 (platform-side)
# Related: Reverted GPUConfig changes in PR #394Rationale: Makes the temporary workaround more traceable and easier to find when the platform fix is deployed. 2. Consider Zero vs None DistinctionThe current conditional Question: Should Current behavior: Recommendation: Keep current behavior unless there's a valid use case for zero-duration. 📊 CI/CD StatusPassing:
Pending:
Skipped:
🎖️ Strengths
🚀 RecommendationApprove and merge with confidence. This is a well-scoped temporary workaround that:
📚 Related Documentation
Reviewed by: Claude Code (Sonnet 4.5) |
Codecov Report✅ All modified and coverable lines are covered by tests. |
|
| } | ||
| } | ||
| # TODO(oliverm): remove this conditional when applications handle null flex_start_max_run_duration_minutes | ||
| if PIPELINE_GPU_FLEX_START_MAX_RUN_DURATION_MINUTES: |
There was a problem hiding this comment.
Would using gpu_config.model_dump(exclude_none=True) with the Pydantic model class be an option to avoid this if block?
There was a problem hiding this comment.
Yes, however that would also exclude other fields which happen to be null, and if any of those are nullable but not optional on the application side, I could end up breaking something. Since this is a temporary fix, I prefer this slightly messier but also slightly less risky approach.
2 participants
We again see some tests falling back to
ON_DEMANDprovisioning after we reverted theGPUConfigchanges in #394. The issue is being addressed on the application side with ETA end of January; in the meantime we temporarily patch the tests.