determined-ai / determined

Determined is an open-source machine learning platform that simplifies distributed training, hyperparameter tuning, experiment tracking, and resource management. Works with PyTorch and TensorFlow.
https://determined.ai
Apache License 2.0
2.93k stars 347 forks source link

fix: wrong notebook idleness payload [MD-447] #9571

Closed ioga closed 3 weeks ago

ioga commented 3 weeks ago

Ticket

MD-447

Description

Notebooks were not terminated due to idleness because the master would parse the payload as having idle: false regardless of what the check_idle.py was trying to report.

I suspect at some point in refactoring the Sessions and related code, this PUT switched from json payload to body, and master just could not parse out the boolean out of it.

Test Plan

  1. Start a notebook with det notebook start --config idle_timeout=5m
  2. wait for notebook to open.
  3. close the notebook. don't do anything in it, especially do not launch kernels! or, if you do, use a different notebook_idle_type value.
  4. wait for 5m
  5. notebook should get terminated

Checklist

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 49.80%. Comparing base (651264b) to head (559ce53). Report is 3 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #9571 +/- ## ======================================= Coverage 49.80% 49.80% ======================================= Files 1247 1247 Lines 162243 162243 Branches 2888 2888 ======================================= Hits 80805 80805 Misses 81266 81266 Partials 172 172 ``` | [Flag](https://app.codecov.io/gh/determined-ai/determined/pull/9571/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | Coverage Δ | | |---|---|---| | [backend](https://app.codecov.io/gh/determined-ai/determined/pull/9571/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | `43.89% <ø> (ø)` | | | [harness](https://app.codecov.io/gh/determined-ai/determined/pull/9571/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | `63.74% <ø> (ø)` | | | [web](https://app.codecov.io/gh/determined-ai/determined/pull/9571/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | `46.16% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#carryforward-flags-in-the-pull-request-comment) to find out more. [see 4 files with indirect coverage changes](https://app.codecov.io/gh/determined-ai/determined/pull/9571/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai)
netlify[bot] commented 3 weeks ago

Deploy Preview for determined-ui canceled.

Name Link
Latest commit 1dbdd0e7bbc3ac37c2909b6c4fb2ff3254b32892
Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/667c4e03ca203b00085ac4a0