Skip to content
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

add: warning when maximum resources to allocate is not enough to the minimum required resources in selected image #2700

Conversation

lizable
Copy link
Contributor

@lizable lizable commented Sep 10, 2024

TL;DR

Added resource allocation warning for insufficient resources and implemented end-to-end test for resource policy modification.

What changed?

  • Added a new warning message for insufficient resource allocation in the session launcher
  • Implemented logic to check if minimum resource values exceed maximum values
  • Added an end-to-end test for modifying resource policy as a superadmin
  • Updated the DynamicUnitInputNumberWithSlider component to handle cases where minimum values exceed maximum values
  • Added translations for the new warning message across multiple languages

How to test?

  1. Log in as a superadmin and navigate to the resource policy page
  2. Modify the resource limits (CPU, memory) to zero
  3. Go to the session start page and check for the warning message in the resource allocation section
  4. Run the new end-to-end test to ensure the resource policy modification works as expected

Why make this change?

This change improves the user experience by providing clear feedback when allocatable resources fall below the minimum required resources for a selected image. It also ensures that the system handles cases where resource limits are set incorrectly, preventing potential errors or unexpected behavior during session creation.

Screenshot(s)

After Before
Screenshot 2024-09-10 at 6.01.58 PM.png Screenshot 2024-09-10 at 2.27.27 PM.png

Checklist: (if applicable)

  • Mention to the original issue
  • Documentation
  • Minium required manager version
  • Specific setting for review (eg., KB link, endpoint or how to setup)
  • Minimum requirements to check during review
  • Test case(s) to demonstrate the difference of before/after

Copy link

graphite-app bot commented Sep 10, 2024

Your org requires the Graphite merge queue for merging into main

Add the label “flow:merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “flow:hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

Copy link
Contributor Author

lizable commented Sep 10, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @lizable and the rest of your teammates on Graphite Graphite

@github-actions github-actions bot added area:ux UI / UX issue. area:i18n Localization size:L 100~500 LoC labels Sep 10, 2024
Copy link

github-actions bot commented Sep 10, 2024

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements 5.41% 338/6246
🔴 Branches 4.93% 214/4342
🔴 Functions 3.07% 63/2053
🔴 Lines 5.31% 324/6101

Test suite run success

90 tests passing in 11 suites.

Report generated by 🧪jest coverage report action from 9821f06

@lizable lizable marked this pull request as ready for review September 11, 2024 02:30
Copy link
Contributor

@ironAiken2 ironAiken2 left a comment

Choose a reason for hiding this comment

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

LGTM

@ironAiken2 ironAiken2 force-pushed the fix/show-warning-when-max-allocatable-resources-fall-short-for-minimum-required-of-image branch from ae6845c to 3700db1 Compare September 20, 2024 01:15
Copy link

graphite-app bot commented Sep 25, 2024

Merge activity

…minimum required resources in selected image (#2700)

### TL;DR

Added resource allocation warning for insufficient resources and implemented end-to-end test for resource policy modification.

### What changed?

- Added a new warning message for insufficient resource allocation in the session launcher
- Implemented logic to check if minimum resource values exceed maximum values
- Added an end-to-end test for modifying resource policy as a superadmin
- Updated the DynamicUnitInputNumberWithSlider component to handle cases where minimum values exceed maximum values
- Added translations for the new warning message across multiple languages

### How to test?

1. Log in as a superadmin and navigate to the resource policy page
2. Modify the resource limits (CPU, memory) to zero
3. Go to the session start page and check for the warning message in the resource allocation section
4. Run the new end-to-end test to ensure the resource policy modification works as expected

### Why make this change?

This change improves the user experience by providing clear feedback when allocatable resources fall below the minimum required resources for a selected image. It also ensures that the system handles cases where resource limits are set incorrectly, preventing potential errors or unexpected behavior during session creation.

### Screenshot(s)
| After | Before |
|------|--------|
| ![Screenshot 2024-09-10 at 6.01.58 PM.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/Dgoz5XqHffJdivyccymr/cf70c2e0-af19-4fd9-8c42-b93f64690529.png) | ![Screenshot 2024-09-10 at 2.27.27 PM.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/Dgoz5XqHffJdivyccymr/f370f7dd-c598-4814-ab46-900adda87a9a.png) |

---

<!--
Please precisely, concisely, and concretely describe what this PR changes, the rationale behind codes,
and how it affects the users and other developers.
-->

**Checklist:** (if applicable)

- [ ] Mention to the original issue
- [ ] Documentation
- [ ] Minium required manager version
- [ ] Specific setting for review (eg., KB link, endpoint or how to setup)
- [ ] Minimum requirements to check during review
- [ ] Test case(s) to demonstrate the difference of before/after
@lizable lizable force-pushed the fix/show-warning-when-max-allocatable-resources-fall-short-for-minimum-required-of-image branch from 3700db1 to 9821f06 Compare September 25, 2024 06:07
@graphite-app graphite-app bot merged commit 9821f06 into main Sep 25, 2024
6 checks passed
@graphite-app graphite-app bot deleted the fix/show-warning-when-max-allocatable-resources-fall-short-for-minimum-required-of-image branch September 25, 2024 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:i18n Localization area:ux UI / UX issue. size:L 100~500 LoC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants