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

Nim work for 2.16 #379

Merged

Conversation

andrewballantyne
Copy link

@andrewballantyne andrewballantyne commented Nov 15, 2024

Last of the NIM work.

Update 2024-11-15, 3:10pm ET: Seems there are some problems with the current makeshift deployment on the NIM cluster -- not sure this code will work on a RHOAI cluster. (thread)
Update 2024-11-15, 4:35pm ET: A fix is found for our issue -- looking to get it into main & then our release branch which will update this PR.
Update 2024-11-15, 5:40pm ET: Last commit has merged successfully through the Pipeline -- waiting on one last CI run and a merge

* feat: added ability to deploy more than one NIM model. (#3453)

* feat: added ability to deploy more than one NIM model.

Signed-off-by: Olga Lavtar <[email protected]>

* feat: refactored NIM related logic to nimUtils

Signed-off-by: Olga Lavtar <[email protected]>

* feat: changes to the error handling

Signed-off-by: Olga Lavtar <[email protected]>

* feat: changes to the error handling

Signed-off-by: Olga Lavtar <[email protected]>

* feat: changes .some for .forEach

Signed-off-by: Olga Lavtar <[email protected]>

* feat: deleting secrets fix

Signed-off-by: Olga Lavtar <[email protected]>

* feat: deploying the same model pvc fix

Signed-off-by: Olga Lavtar <[email protected]>

* feat: added a unit test for getNIMResourcesToDelete

Signed-off-by: Olga Lavtar <[email protected]>

* feat: will add a unit test for getNIMResourcesToDelete later

Signed-off-by: Olga Lavtar <[email protected]>

---------

Signed-off-by: Olga Lavtar <[email protected]>

* feat: Modify NIM enablement process (#3455)

* Modify NIM enablement process

* Clean up code and remove unnecessary manifests

* Add logic to check the conditions of the odh-nim-account CR

* Added more check for enabled integration apps

* If failed to fetch integration app status, should show error on the related tile in the enable application page, should not remove the tile

* check integration app status in explore application page

* Fix lint issue

* Fix lint issue

* add annotations to the secret and the account CR

* Update backend/src/routes/api/integrations/nim/index.ts

Co-authored-by: Andrew Ballantyne <[email protected]>

* Update backend/src/routes/api/components/list.ts

Co-authored-by: Andrew Ballantyne <[email protected]>

* clean up

* feat: listing all NIM accounts in the namespace, returning the first one.

Signed-off-by: Olga Lavtar <[email protected]>

* Avoid mutating object in useWatchIntegrationComponents

* Clean up

* feat: added logic for displaying the tile correctly

Signed-off-by: Olga Lavtar <[email protected]>

* feat: changes for enabling

Signed-off-by: Olga Lavtar <[email protected]>

* feat: updated mock component with the new properties.

Signed-off-by: Olga Lavtar <[email protected]>

* feat: addressed PR comments with updates and improvements

Signed-off-by: Olga Lavtar <[email protected]>

* feat: backend bug fix

Signed-off-by: Olga Lavtar <[email protected]>

* feat: Missed change from previous commit

Signed-off-by: Olga Lavtar <[email protected]>

* feat: fix for the enabled page and enabled.cy.ts

Signed-off-by: Olga Lavtar <[email protected]>

---------

Signed-off-by: Olga Lavtar <[email protected]>
Co-authored-by: Andrew Ballantyne <[email protected]>
Co-authored-by: Olga Lavtar <[email protected]>

* Fix NIM selection issue (#3482)

* Fix NIM selection issue

* Switch back to using a numerical value

---------

Signed-off-by: Olga Lavtar <[email protected]>
Co-authored-by: olavtar <[email protected]>
Co-authored-by: yu zhao <[email protected]>
Co-authored-by: Olga Lavtar <[email protected]>
@andrewballantyne andrewballantyne changed the title [WIP] Nim work for 2.16 (#3484) [WIP] Nim work for 2.16 Nov 15, 2024
@andrewballantyne andrewballantyne changed the title [WIP] Nim work for 2.16 [DO NOT MERGE] Nim work for 2.16 Nov 15, 2024
@codecov-commenter
Copy link

codecov-commenter commented Nov 15, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 47.80876% with 131 lines in your changes missing coverage. Please review.

Project coverage is 85.41%. Comparing base (b09ef1b) to head (23233c1).
Report is 4 commits behind head on rhoai-2.16.

Files with missing lines Patch % Lines
frontend/src/utilities/useEnableApplication.tsx 1.69% 58 Missing ⚠️
...nd/src/utilities/useWatchIntegrationComponents.tsx 37.73% 33 Missing ⚠️
frontend/src/services/integrationAppService.ts 13.33% 13 Missing ⚠️
...rc/pages/modelServing/screens/projects/nimUtils.ts 60.00% 12 Missing ⚠️
...ing/screens/global/DeleteInferenceServiceModal.tsx 77.77% 4 Missing ⚠️
...d/src/pages/modelServing/screens/projects/utils.ts 60.00% 4 Missing ⚠️
...projects/NIMServiceModal/ManageNIMServingModal.tsx 78.57% 3 Missing ⚠️
...d/src/pages/exploreApplication/GetStartedPanel.tsx 60.00% 2 Missing ⚠️
...pages/exploreApplication/useIntegratedAppStatus.ts 87.50% 1 Missing ⚠️
...lServing/screens/projects/ModelServingPlatform.tsx 92.85% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff               @@
##           rhoai-2.16     #379      +/-   ##
==============================================
- Coverage       85.65%   85.41%   -0.24%     
==============================================
  Files            1347     1350       +3     
  Lines           30676    30827     +151     
  Branches         8554     8592      +38     
==============================================
+ Hits            26274    26332      +58     
- Misses           4402     4495      +93     
Files with missing lines Coverage Δ
frontend/src/concepts/areas/const.ts 100.00% <ø> (ø)
.../pages/enabledApplications/EnabledApplications.tsx 100.00% <100.00%> (ø)
...ntend/src/pages/exploreApplication/EnableModal.tsx 32.07% <ø> (ø)
.../modelServing/screens/global/EmptyModelServing.tsx 78.57% <100.00%> (-4.77%) ⬇️
...modelServing/screens/projects/useIsNIMAvailable.ts 100.00% <100.00%> (ø)
frontend/src/pages/modelServing/screens/types.ts 100.00% <ø> (ø)
...c/pages/modelServing/useServingPlatformStatuses.ts 100.00% <100.00%> (ø)
...il/overview/serverModels/PlatformSelectSection.tsx 100.00% <100.00%> (ø)
...etail/overview/serverModels/ServeModelsSection.tsx 100.00% <100.00%> (ø)
...verModels/deployedModels/DeployedModelsSection.tsx 94.64% <100.00%> (-0.10%) ⬇️
... and 12 more

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b09ef1b...23233c1. Read the comment docs.

* Improve error message in log

* Fix the Service Account permissions
Copy link

sonarcloud bot commented Nov 15, 2024

@andrewballantyne andrewballantyne changed the title [DO NOT MERGE] Nim work for 2.16 Nim work for 2.16 Nov 15, 2024
@andrewballantyne andrewballantyne merged commit f2a6640 into red-hat-data-services:rhoai-2.16 Nov 15, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants