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

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

Merged
merged 3 commits into from
Jun 26, 2024
Merged

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

merged 3 commits into from
Jun 26, 2024

Conversation

ioga
Copy link
Contributor

@ioga ioga commented Jun 25, 2024

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

  • 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

@ioga ioga added the to-cherry-pick Pull requests that need to be cherry-picked into the current release label Jun 25, 2024
@ioga ioga requested review from a team and jgongd and removed request for a team June 25, 2024 23:07
@ioga ioga requested a review from a team as a code owner June 25, 2024 23:07
@cla-bot cla-bot bot added the cla-signed 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 (651264b) to head (559ce53).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           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 Coverage Δ
backend 43.89% <ø> (ø)
harness 63.74% <ø> (ø)
web 46.16% <ø> (ø)

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

see 4 files with indirect coverage changes

@determined-ci determined-ci requested a review from a team June 25, 2024 23:07
@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 1dbdd0e
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/667c4e03ca203b00085ac4a0

@ioga ioga requested a review from a team June 25, 2024 23:10
Copy link
Contributor

@jgongd jgongd left a comment

Choose a reason for hiding this comment

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

Work as expected

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

@determined-ci determined-ci requested a review from a team June 26, 2024 17:21
@ioga ioga merged commit e5c5768 into main Jun 26, 2024
15 of 64 checks passed
@ioga ioga deleted the fix-idle-timeout branch June 26, 2024 17:22
github-actions bot 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.

4 participants