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

Support for new status and forceclose server commands #229

Merged
merged 32 commits into from
Mar 14, 2025

Conversation

jkrumbiegel
Copy link
Collaborator

@jkrumbiegel jkrumbiegel commented Jan 6, 2025

Adds a new status message for the socket server which sends back metadata about the server and active workers, to be shown via the planned quarto call engine julia status command.

These commands are meant to work with this quarto-cli PR: quarto-dev/quarto-cli#11803

This PR changes the behavior of run, close and stop such that they error when they are used on files that are currently being run. Before, the command tasks would just wait until they could attain file locks, which in the case of a very long running or hung worker process could take forever. This PR adds the forceclose command to forcibly close a running file in those cases.

Example status

$ quarto call engine julia status                
QuartoNotebookRunner server status:
  started at: 10:59:19 (48 minutes 38 seconds ago)
  runner version: 0.14.0
  environment: /Users/krumbiegel/.julia/dev/QuartoNotebookRunner/revise/
  pid: 64127
  port: 8001
  julia version: 1.11.4
  timeout: 5 minutes
  workers active: 1
    worker 1:
      path: /Users/krumbiegel/.julia/dev/QuartoNotebookRunner/scratch/test.qmd
      run started: 10:59:27 (48 minutes 30 seconds ago)
      run finished: -
      timeout: 5 minutes
      pid: 64141
      exe: `julia`
      exeflags: ["--color=yes"]
      env: ["JULIA_PROJECT=@."]

Example forceclose

This is currently exposed as close --force in the quarto-cli PR. First, normal close throws an error now:

$ quarto call engine julia close scratch/test.qmd
ERROR: Julia server returned error after receiving "close" command:

Failed to close notebook: /Users/krumbiegel/.julia/dev/QuartoNotebookRunner/scratch/test.qmd

The underlying Julia error was:

Tried to close file "/Users/krumbiegel/.julia/dev/QuartoNotebookRunner/scratch/test.qmd" but the corresponding worker is busy.

Stack trace:
    at writeJuliaCommand (file:///Users/krumbiegel/dev/quarto-cli/src/execute/julia.ts:790:13)
    at eventLoopTick (ext:core/01_core.js:175:7)
    at async connectAndWriteJuliaCommandToRunningServer (file:///Users/krumbiegel/dev/quarto-cli/src/execute/julia.ts:957:20)
    at async closeWorker (file:///Users/krumbiegel/dev/quarto-cli/src/execute/julia.ts:974:3)
    at async Command.actionHandler (file:///Users/krumbiegel/dev/quarto-cli/src/execute/julia.ts:875:7)
    at async Command.execute (https://deno.land/x/[email protected]/command/command.ts:1948:7)
    at async Command.parseCommand (https://deno.land/x/[email protected]/command/command.ts:1780:14)
    at async quarto (file:///Users/krumbiegel/dev/quarto-cli/src/quarto.ts:191:5)
    at async file:///Users/krumbiegel/dev/quarto-cli/src/quarto.ts:220:5
    at async file:///Users/krumbiegel/dev/quarto-cli/src/core/main.ts:41:14

But force-closing works:

$ quarto call engine julia close --force scratch/test.qmd
Worker force-closed successfully.

Copy link

codecov bot commented Jan 6, 2025

Codecov Report

Attention: Patch coverage is 87.82051% with 19 lines in your changes missing coverage. Please review.

Project coverage is 35.28%. Comparing base (53ce1e1) to head (e1d047b).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/socket.jl 87.09% 12 Missing ⚠️
src/server.jl 88.88% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #229      +/-   ##
==========================================
+ Coverage   32.09%   35.28%   +3.18%     
==========================================
  Files          57       57              
  Lines        2673     2814     +141     
==========================================
+ Hits          858      993     +135     
- Misses       1815     1821       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jkrumbiegel jkrumbiegel changed the title Worker status Server status Mar 10, 2025
@jkrumbiegel jkrumbiegel changed the title Server status Support for new status and forceclose server commands Mar 12, 2025
@jkrumbiegel jkrumbiegel marked this pull request as ready for review March 12, 2025 10:56
@MichaelHatherly
Copy link
Collaborator

Logic seems solid to me at a first look. I'll try and actually run some stuff later today if I have some time.

@jkrumbiegel jkrumbiegel merged commit ad55690 into main Mar 14, 2025
11 checks passed
@jkrumbiegel jkrumbiegel deleted the jk/workers-status branch March 14, 2025 19:30
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.

2 participants