Min max deployment improvements #4632
Conversation
WalkthroughAdds cross-field validation and inline warnings to min/max replica inputs, exposes validation state via onValidationChange up the component chain, renames/refactors ReplicaSection, removes most auto-sync between min/max, updates modal submit gating, and adds Cypress tests and page-object finders for the new messages. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant RS as ReplicaSection
participant KA as KServeAutoscalerReplicaSection
participant MM as ManageKServeModal
U->>RS: Edit min/max inputs
RS->>RS: Validate, clamp, set editing/empty flags
RS-->>KA: onValidationChange(hasErrors)
KA-->>MM: onValidationChange(hasErrors)
MM->>MM: set hasReplicaValidationErrors
MM-->>U: Disable/enable Submit based on errors
sequenceDiagram
participant U as User
participant RS as ReplicaSection
participant KA as KServeAutoscalerReplicaSection
U->>RS: Change Min
RS-->>KA: onMinChange(newMin)
alt isKServeRawDeployment
KA->>KA: may update Max to match Min
else
KA-->>KA: no auto-sync to Max
end
U->>RS: Change Max
KA-->>KA: no auto-sync to Min
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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). (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
ashley-o0o
requested review from
Griffin-Sullivan,
emilys314 and
katieperry4
and removed request for
manaswinidas and
pnaik1
ashley-o0o
force-pushed
the
26252/min-max-deployment-improvements
branch
from
8a8f719 to
32add70
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4632 +/- ##
===========================================
- Coverage 82.16% 29.23% -52.94%
===========================================
Files 1846 1854 +8
Lines 38533 38504 -29
Branches 11480 11815 +335
===========================================
- Hits 31661 11257 -20404
- Misses 6872 27247 +20375
... and 1606 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
/test-e2e |
|
E2E Tests #2484 triggered by #4632 (comment) |
|
E2E Tests #2484 finished with result: UNSTABLE |
|
Would it be possible to disable the deploy button if the warning message is showing? Otherwise everything looks great! The leading |
|
Could we replicate these changes for the min > max? Seems like the old ux is present when changing the min |
|
Updated to disable the deploy button when editing numbers and the warning message is showing. Also modifies the minimum replica field to work the same as the maximum. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/src/components/ReplicaSection.tsx (3)
41-46: Reduce duplication: use a single “out-of-order” predicate
maxLessThanMinandminGreaterThanMaxare equivalent. Consider consolidating to improve readability and prevent drift.Apply:
- const maxLessThanMin = !showMinMax && maxValue < value; - const minGreaterThanMax = !showMinMax && value > maxValue; - const showMinWarning = minGreaterThanMax && editingField === 'min'; - const showMaxWarning = maxLessThanMin && editingField === 'max'; + const outOfOrder = !showMinMax && value > maxValue; + const showMinWarning = outOfOrder && editingField === 'min'; + const showMaxWarning = outOfOrder && editingField === 'max';
76-96: Guardrails look correct, but consider explicit empty input handlingUsing
val === undefined ? 0 : Number(val)makes cleared inputs immediately become0. That’s fine given requirements, but you might prefer to preserve “empty” during editing and only coerce on blur for a slightly smoother UX (optional).If you want this behavior, I can propose a wrapper change that passes through
undefinedwhile editing and coerces on blur only.
146-175: Optional: add data-testid on helper texts for more stable e2e assertionsCypress currently locates these by text via page object helpers. Adding dedicated testids will make tests resilient to minor copy changes and reduce flakiness.
Example:
- <FormHelperText + <FormHelperText + data-testid="min-replicas-error" style={{ whiteSpace: 'nowrap' }} > <HelperText> <HelperTextItem icon={<ExclamationTriangleIcon />} variant="warning"> Minimum replicas must be less than or equal to maximum replicas. </HelperTextItem> </HelperText> </FormHelperText> ... - <FormHelperText + <FormHelperText + data-testid="max-replicas-error" style={{ whiteSpace: 'nowrap' }} > <HelperText> <HelperTextItem icon={<ExclamationTriangleIcon />} variant="warning"> Maximum replicas must be greater than or equal to minimum replicas. </HelperTextItem> </HelperText> </FormHelperText>I can follow up with a small PR to update the Cypress page helpers to use these selectors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (5)
frontend/src/__tests__/cypress/cypress/pages/modelServing.ts(1 hunks)frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/runtime/servingRuntimeList.cy.ts(2 hunks)frontend/src/components/ReplicaSection.tsx(5 hunks)frontend/src/pages/modelServing/screens/projects/kServeModal/KServeAutoscalerReplicaSection.tsx(2 hunks)frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/tests/cypress/cypress/pages/modelServing.ts
- frontend/src/pages/modelServing/screens/projects/kServeModal/KServeAutoscalerReplicaSection.tsx
🧰 Additional context used
📓 Path-based instructions (1)
frontend/src/__tests__/cypress/cypress/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/cypress-e2e.mdc)
frontend/src/__tests__/cypress/cypress/**/*.{ts,tsx}: Don't create TypeScript interfaces for test data; use direct object access patterns.
All linting errors must be fixed: use object destructuring, proper formatting, no unused variables/imports, no any types unless necessary.
Never hardcode namespaces in tests or utilities; always derive namespaces from test variables or environment variables.
Files:
frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/runtime/servingRuntimeList.cy.ts
🧬 Code Graph Analysis (2)
frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/runtime/servingRuntimeList.cy.ts (1)
frontend/src/__tests__/cypress/cypress/pages/modelServing.ts (1)
kserveModal(846-846)
frontend/src/components/ReplicaSection.tsx (1)
frontend/src/utilities/utils.ts (1)
normalizeBetween(152-162)
⏰ 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). (2)
- GitHub Check: Lint
- GitHub Check: Cypress-Setup
🔇 Additional comments (6)
frontend/src/components/ReplicaSection.tsx (1)
40-47: Good addition: local editing context drives symmetric min/max validation and parent callbackThe editing-scoped warnings, snap-on-blur behavior, and
onValidationChangecallback fulfill the PR goals (keyboard editing, symmetric warnings, blur snap, and disabling submit while warning is active). This is a sensible UX and state split.frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/runtime/servingRuntimeList.cy.ts (2)
1925-1937: E2E coverage for min/max warnings is spot onThese checks validate the UX guarantees: error shown when max < min or min > max, and cleared on correction without requiring blur. This mirrors the component’s editing-scoped validation.
2003-2005: Double-check expected min/max (40/40) are intended defaults for this scenarioThe request body expectations assert
minReplicas: 40andmaxReplicas: 40. Given earlier interactions toggle between Advanced and Standard modes and don’t explicitly set 40 in the UI here, please confirm this is the intended outcome (likely from updated defaults or test data).If you want, I can scan the codebase to find where 40 is set as the default for raw/standard modes and confirm this path drives the expected payload.
frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx (3)
134-135: Correct: track replica validation errors and wire through to disable submitThis links the new validation surface to the modal submission guard as requested in PR comments. Good integration point.
431-437: Propagating validation via onValidationChange is clean and keeps concerns separatedForwarding the callback to the underlying section allows the autoscaler component to remain the source of truth for min/max validity. Nice separation of concerns.
506-508: Submit button gating now correctly includes replica validation stateThis addresses the “deploy succeeds without blur” feedback by disabling submit while the warning is displayed. Behavior matches the PR objective.
|
/test-e2e |
|
E2E Tests #2579 triggered by #4632 (comment) |
There was a problem hiding this comment.
To answer @katieperry4 's points there is probably a way to trim the leading zero as we edit the field but everything I tried to change wasn't working so I'm just confused now haha. Other than the component name everything looks good to me.
|
E2E Tests #2579 finished with result: UNSTABLE |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/src/components/ReplicaSection.tsx (1)
30-30: Component naming reverted to ReplicaSection — resolves earlier reviewRenaming back to ReplicaSection and exporting it as default addresses the prior feedback.
Also applies to: 180-180
🧹 Nitpick comments (4)
frontend/src/components/ReplicaSection.tsx (4)
41-50: Consolidate duplicated comparisons and compute validation state oncemaxLessThanMin and minGreaterThanMax are logically equivalent; you can reduce to a single flag and derive everything else. Also, hasValidationErrors can be expressed in terms of that single flag. This cuts redundant state and clarifies intent.
Apply this refactor:
- const maxLessThanMin = !showMinMax && maxValue < value; - const minGreaterThanMax = !showMinMax && value > maxValue; - const showMinWarning = minGreaterThanMax && editingField === 'min'; - const showMaxWarning = maxLessThanMin && editingField === 'max'; - - const hasValidationErrors = showMinWarning || showMaxWarning; + const replicasInvalid = !showMinMax && value > maxValue; + const showMinWarning = replicasInvalid && editingField === 'min'; + const showMaxWarning = replicasInvalid && editingField === 'max'; + + const hasValidationErrors = replicasInvalid && editingField !== null;
89-95: Let normalizer clamp instead of rejecting out-of-range keystrokesCurrently, onChange ignores values > upperLimit (e.g., typing “100” does nothing). Consider always normalizing/clamping and updating, which simplifies logic and avoids “ignored keystroke” UX. You still retain the hard cap of 99.
Apply across the three handlers:
- if (!Number.isNaN(newSize) && newSize <= upperLimit) { + if (!Number.isNaN(newSize)) { onChange(normalizeBetween(newSize, lowerLimit, upperLimit)); }- if (!Number.isNaN(newSize) && newSize <= upperLimit) { + if (!Number.isNaN(newSize)) { onChange(normalizeBetween(newSize, lowerLimit, upperLimit)); }- if (!Number.isNaN(newSize) && newSize <= upperLimit) { + if (!Number.isNaN(newSize)) { onMaxChange(normalizeBetween(newSize, lowerLimit, upperLimit)); }Also applies to: 104-110, 134-140
119-119: Inline style may be unnecessarystyle={{ position: 'relative' }} on the max FormGroup doesn’t appear to influence layout since helper text is rendered after the Flex container. If not strictly needed, consider removing to reduce styling noise.
146-175: Add aria-live to inline warnings for SR accessibilityTo ensure screen readers announce the dynamic warning text as it appears, add aria-live="polite" to the FormHelperText containers.
- <FormHelperText + <FormHelperText + aria-live="polite" style={{ whiteSpace: 'nowrap', }} >- <FormHelperText + <FormHelperText + aria-live="polite" style={{ whiteSpace: 'nowrap', }} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
frontend/src/components/ReplicaSection.tsx(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/components/ReplicaSection.tsx (1)
frontend/src/utilities/utils.ts (1)
normalizeBetween(152-162)
⏰ 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). (2)
- GitHub Check: Cypress-Setup
- GitHub Check: Lint
🔇 Additional comments (3)
frontend/src/components/ReplicaSection.tsx (3)
2-12: PF imports for inline warnings look goodBringing in FormHelperText/HelperText and ExclamationTriangleIcon is appropriate for inline validation messaging. No concerns here.
Also applies to: 12-12
81-88: Button-disable guards are correct and prevent crossing constraints
- Disabling the min plus button when min >= max prevents overshooting.
- Disabling the max minus button when max <= min prevents undershooting.
This directly addresses the UX requirement while allowing free-form typing.
Also applies to: 126-133
27-50: Validation wiring OK — E2E coverage unclearVerified the wiring:
- frontend/src/components/ReplicaSection.tsx — calls onValidationChange(hasValidationErrors) via useEffect.
- frontend/src/pages/modelServing/screens/projects/kServeModal/KServeAutoscalerReplicaSection.tsx — forwards onValidationChange to ReplicaSection (onValidationChange={onValidationChange}).
- frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx — passes setHasReplicaValidationErrors and includes hasReplicaValidationErrors in the isSubmitDisabled condition.
Action required:
- I couldn't find Cypress/E2E tests that explicitly assert the transient warning scenarios (min > max and max < min while editing, without blur). Please add tests that cover both cases (ensure submit is disabled while the warning is visible and re-enables after blur) or point to the existing tests that already cover them.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
frontend/src/components/ReplicaSection.tsx (2)
30-39: Rename resolved: component is exported as ReplicaSectionThis addresses the previous review note about naming. No further action needed.
194-194: Default export name matches componentConsistent export; aligns with prior naming feedback.
🧹 Nitpick comments (7)
frontend/src/components/ReplicaSection.tsx (7)
40-49: Simplify duplicated invalid-range checksBoth
maxLessThanMinandminGreaterThanMaxare equivalent (mirror statements). Collapse to a singleisInvalidRangefor clarity and to avoid drift on future edits.Apply this diff:
- const maxLessThanMin = !showMinMax && maxValue < value; - const minGreaterThanMax = !showMinMax && value > maxValue; - const showMinWarning = minGreaterThanMax && editingField === 'min'; - const showMaxWarning = maxLessThanMin && editingField === 'max'; + const isInvalidRange = !showMinMax && value > maxValue; + const showMinWarning = isInvalidRange && editingField === 'min'; + const showMaxWarning = isInvalidRange && editingField === 'max';
79-91: Improve UX: treat 0 as empty on focus to avoid confusing leading zeroReview feedback mentioned a confusing leading “0”. When focusing the min input with a value of 0, pre-clear to make typing smoother without fighting the leading digit. This does not change the underlying value until blur or a valid change occurs.
Apply this diff:
- onFocus={() => setEditingField('min')} + onFocus={() => { + setEditingField('min'); + if (value === 0) { + setIsMinEmpty(true); + } + }}
92-103: Let normalizeBetween handle upper bound clamping (avoid gating on <= upperLimit)Current check blocks updates when users type above 99 instead of clamping to 99, causing keystrokes to be ignored. Prefer clamping for a smoother, predictable UX.
Apply this diff:
- if (!Number.isNaN(newSize) && newSize <= upperLimit) { - onChange(normalizeBetween(newSize, lowerLimit, upperLimit)); - } + if (!Number.isNaN(newSize)) { + onChange(normalizeBetween(newSize, lowerLimit, upperLimit)); + }
127-127: Nit: Inline style on FormGroup may be unnecessary
style={{ position: 'relative' }}on the max FormGroup isn’t used by the current inline warnings (rendered outside the group). If this was left for a future overlay/popover, add a brief comment. Otherwise, consider removing it.
130-142: Mirror the “leading zero” UX for max inputApply the same “0 as empty on focus” behavior to the max input for consistency.
Apply this diff:
- onFocus={() => setEditingField('max')} + onFocus={() => { + setEditingField('max'); + if (maxValue === 0) { + setIsMaxEmpty(true); + } + }}
143-154: Let normalizeBetween clamp max without manual upper bound gateSame rationale as the min input: prefer clamping to 99 instead of ignoring keystrokes above 99.
Apply this diff:
- if (!Number.isNaN(newSize) && newSize <= upperLimit) { - onMaxChange(normalizeBetween(newSize, lowerLimit, upperLimit)); - } + if (!Number.isNaN(newSize)) { + onMaxChange(normalizeBetween(newSize, lowerLimit, upperLimit)); + }
160-189: Add test ids and aria-live to warnings for robust E2E and better a11yCypress tests that rely on text can be brittle. Test IDs make them resilient. Also consider
aria-live="polite"so SR users are informed when the message appears.Apply this diff:
- {showMinWarning && ( - <FormHelperText + {showMinWarning && ( + <FormHelperText + data-testid="min-replicas-error" + aria-live="polite" style={{ whiteSpace: 'nowrap', }} > <HelperText> <HelperTextItem icon={<ExclamationTriangleIcon />} variant="warning"> Minimum replicas must be less than or equal to maximum replicas. </HelperTextItem> </HelperText> </FormHelperText> )} - {showMaxWarning && ( - <FormHelperText + {showMaxWarning && ( + <FormHelperText + data-testid="max-replicas-error" + aria-live="polite" style={{ whiteSpace: 'nowrap', }} > <HelperText> <HelperTextItem icon={<ExclamationTriangleIcon />} variant="warning"> Maximum replicas must be greater than or equal to minimum replicas. </HelperTextItem> </HelperText> </FormHelperText> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
frontend/src/components/ReplicaSection.tsx(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/components/ReplicaSection.tsx (1)
frontend/src/utilities/utils.ts (1)
normalizeBetween(152-162)
⏰ 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). (2)
- GitHub Check: Cypress-Setup
- GitHub Check: Lint
🔇 Additional comments (3)
frontend/src/components/ReplicaSection.tsx (3)
2-12: LGTM on expanded imports to support warning UXThe added PatternFly helper text components and warning icon are appropriate for the inline validation UI.
27-27: Public API extension (onValidationChange) is clear and usefulOptional callback to surface validation state upward is the right hook for disabling the Deploy action while warnings are visible.
50-53: Verified — Deploy button is disabled while replica validation warnings are presentWiring confirmed end-to-end: ReplicaSection emits validation state, KServeAutoscalerReplicaSection forwards the prop, and ManageKServeModal uses the state to disable the submit.
- frontend/src/components/ReplicaSection.tsx — useEffect calls
onValidationChange?.(hasValidationErrors)(lines ~50–52).- frontend/src/pages/modelServing/screens/projects/kServeModal/KServeAutoscalerReplicaSection.tsx — accepts
onValidationChangeand passesonValidationChange={onValidationChange}intoReplicaSection(lines ~10, 17, 34).- frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx — sets
const [hasReplicaValidationErrors, setHasReplicaValidationErrors] = React.useState(false);, passesonValidationChange={setHasReplicaValidationErrors}to the section (line ~436), and includeshasReplicaValidationErrorsinDashboardModalFooter'sisSubmitDisabled(line ~507).No changes required.
ashley-o0o
force-pushed
the
26252/min-max-deployment-improvements
branch
from
078cd8e to
c417741
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/runtime/servingRuntimeList.cy.ts(2 hunks)frontend/src/components/ReplicaSection.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/ReplicaSection.tsx
🧰 Additional context used
📓 Path-based instructions (1)
frontend/src/__tests__/cypress/cypress/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/cypress-e2e.mdc)
frontend/src/__tests__/cypress/cypress/**/*.{ts,tsx}: Don't create TypeScript interfaces for test data; use direct object access patterns.
All linting errors must be fixed: use object destructuring, proper formatting, no unused variables/imports, no any types unless necessary.
Never hardcode namespaces in tests or utilities; always derive namespaces from test variables or environment variables.
Files:
frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/runtime/servingRuntimeList.cy.ts
🧬 Code Graph Analysis (1)
frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/runtime/servingRuntimeList.cy.ts (1)
frontend/src/__tests__/cypress/cypress/pages/modelServing.ts (1)
kserveModal(846-846)
⏰ 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). (2)
- GitHub Check: Cypress-Setup
- GitHub Check: Lint
🔇 Additional comments (1)
frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/runtime/servingRuntimeList.cy.ts (1)
2003-2005: Confirm that 4/4 min/max in request body is stable after setting max to 99 and switching to “Standard”.You assert
minReplicas: 4andmaxReplicas: 4, but earlier in this test you setmaxto99(by entering100and clamping) and then switch the mode to “Standard” without explicitly setting the Standard-mode replicas. If “Standard” mode derives its single replicas value from the last min (4), this passes; if it derives from max (99) or resets, this can become brittle.
- Consider explicitly setting and asserting the Standard-mode replicas to 4 just after switching to “Standard” to avoid flakiness.
- If the page object doesn’t yet expose a Standard-mode replicas input finder, adding one will make this robust and readable.
Example (to add right after selecting “Standard”):
// After switching to "Standard" kserveModal.findModelServerReplicasInput().clear().type('4'); kserveModal.findModelServerReplicasInput().should('have.value', '4');
...end/src/__tests__/cypress/cypress/tests/mocked/modelServing/runtime/servingRuntimeList.cy.ts
Outdated
Show resolved
Hide resolved
ashley-o0o
force-pushed
the
26252/min-max-deployment-improvements
branch
from
c417741 to
296840f
Compare
|
/test-e2e@Modelserving |
|
E2E Tests #2583 triggered by #4632 (comment) |
|
E2E Tests #2583 finished with result: UNSTABLE |
|
/test-e2e@Modelserving |
|
E2E Tests #2596 triggered by #4632 (comment) |
|
E2E Tests #2596 finished with result: UNSTABLE |
|
at this point the e2e run here is unreliable so let's just move forward with manual testing |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Griffin-Sullivan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit d110aa2
into
opendatahub-io:main
Closes: https://issues.redhat.com/browse/RHOAIENG-26252
Description
Updates how the server replica deployment min/max works. Before, it was difficult to edit the max using your keyboard, limiting you to use the - / + buttons. Now you're able to edit the max using your keyboard much easier. You're able to properly backspace and modify the number.
There is now a popup that will warn you if you're trying to set a number to max that's lower than min, and when you click away it will snap to the min. Edit: It will now also show the warning if you try to set the min to be higher than the max, and will snap to the max when you click away. It will also now disable the deploy button when the warning is being shown.
How Has This Been Tested?
Tested locally.
Test Impact
Modified cypress test to check for error message
Screen Record
Before:
Screen.Recording.2025-08-06.at.10.27.26.AM.mov
After:
Screen.Recording.2025-08-14.at.10.43.04.AM.mov
Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
mainSummary by CodeRabbit
New Features
Bug Fixes
Tests