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

chore: add Nvidia drivers version matching test and bump env [MD-413] #9567

Merged
merged 5 commits into from
Jun 26, 2024

Conversation

jgongd
Copy link
Contributor

@jgongd jgongd commented Jun 25, 2024

Ticket

Description

In order for fabric manager to work properly, it will need to match the version of the Nvidia driver. Added a test in e2e_tests/tests/environment/test_nvidia_driver.py to make sure. The rest of the changes are bump env.

The related environments repo PR: determined-ai/environments#269

Test Plan

CI tests pass.

Checklist

  • Changes have been manually QA'd
  • New features have been approved by the corresponding PM
  • User-facing API changes have the "User-facing API Change" label
  • Release notes have been added as a separate file under docs/release-notes/
    See Release Note for details.
  • Licenses have been included for new code which was copied and/or modified from any external code

@cla-bot cla-bot bot added the cla-signed label Jun 25, 2024
@determined-ci determined-ci requested a review from a team June 25, 2024 17:32
@determined-ci determined-ci added the documentation Improvements or additions to documentation label Jun 25, 2024
Copy link

netlify bot commented Jun 25, 2024

Deploy Preview for determined-ui canceled.

Name Link
🔨 Latest commit bac4522
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/667c80257f109d0008321911

@determined-ci determined-ci removed the documentation Improvements or additions to documentation label Jun 25, 2024
Copy link

codecov bot commented Jun 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.80%. Comparing base (da9025b) to head (bac4522).
Report is 13 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #9567   +/-   ##
=======================================
  Coverage   49.79%   49.80%           
=======================================
  Files        1247     1247           
  Lines      162235   162235           
  Branches     2888     2887    -1     
=======================================
+ Hits        80793    80794    +1     
+ Misses      81270    81269    -1     
  Partials      172      172           
Flag Coverage Δ
backend 43.87% <100.00%> (-0.01%) ⬇️
harness 63.75% <100.00%> (+<0.01%) ⬆️
web 46.16% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
harness/determined/deploy/gcp/constants.py 100.00% <100.00%> (ø)
master/internal/config/provconfig/aws_config.go 11.20% <ø> (ø)
master/internal/config/provconfig/gcp_config.go 26.76% <100.00%> (ø)

... and 2 files with indirect coverage changes

@determined-ci determined-ci added the documentation Improvements or additions to documentation label Jun 25, 2024
@jgongd jgongd requested a review from MikhailKardash June 25, 2024 21:13
@jgongd jgongd marked this pull request as ready for review June 25, 2024 21:14
@jgongd jgongd requested review from a team as code owners June 25, 2024 21:14
Copy link
Contributor

@tara-hpe tara-hpe left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -0,0 +1,37 @@
import re
Copy link
Contributor

Choose a reason for hiding this comment

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

Make sure to add this new directory to the CODEOWNERS file.

Copy link
Contributor

@MikhailKardash MikhailKardash left a comment

Choose a reason for hiding this comment

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

Needs another bumpenv after determined-ai/environments#269 is merged.

@determined-ci determined-ci removed the documentation Improvements or additions to documentation label Jun 26, 2024
@determined-ci determined-ci added the documentation Improvements or additions to documentation label Jun 26, 2024
@determined-ci determined-ci requested a review from a team June 26, 2024 20:53
Copy link
Contributor

@MikhailKardash MikhailKardash left a comment

Choose a reason for hiding this comment

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

LGTM

@jgongd jgongd merged commit d187243 into main Jun 26, 2024
84 of 98 checks passed
@jgongd jgongd deleted the jgong/bumpenv branch June 26, 2024 21:26
@jgongd jgongd added the to-cherry-pick Pull requests that need to be cherry-picked into the current release label Jun 26, 2024
keita-determined pushed a commit that referenced this pull request Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed documentation Improvements or additions to documentation to-cherry-pick Pull requests that need to be cherry-picked into the current release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants