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

Update notebook execution timer #13366

Merged
merged 2 commits into from
Feb 8, 2024
Merged

Conversation

msujew
Copy link
Member

@msujew msujew commented Feb 7, 2024

What it does

Does essentially as the title says. In vscode, the execution time display is updated every 100ms to show that the execution is still running. We just do the same.

How to test

  1. Open a notebook and select a kernel
  2. Start a long running execution, see:
import time
print('1')

time.sleep(2)

print('2')

time.sleep(2)

print('3')
  1. The timer should behave as expected

Review checklist

Reminder for reviewers

@msujew msujew added the notebook issues related to notebooks label Feb 7, 2024
@msujew msujew requested a review from jonah-iden February 7, 2024 16:23
Copy link
Contributor

@jonah-iden jonah-iden left a comment

Choose a reason for hiding this comment

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

out of some reason its starting the counter at 2 seconds when executing a cell

theia_notebook-timer.mp4

so in this example the timer ends 2 seconds after the actual execution time

Copy link
Contributor

@jonah-iden jonah-iden left a comment

Choose a reason for hiding this comment

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

The issue is probably that the clock on serverside has a bit of a differnent time than the one on the client machine.
Maybe when first starting the execution, one could calculate this difference and add that to the time when counting up. But i think this issue is negligible. So approved

Copy link
Contributor

@jonah-iden jonah-iden left a comment

Choose a reason for hiding this comment

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

works like a charm now

@msujew msujew merged commit 913eb3e into master Feb 8, 2024
14 checks passed
@msujew msujew deleted the msujew/update-notebook-execution-timer branch February 8, 2024 12:33
@github-actions github-actions bot added this to the 1.47.0 milestone Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notebook issues related to notebooks
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants