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

Bump QuartoNotebookRunner.jl to 0.11.5 #11100

Merged
merged 2 commits into from
Oct 23, 2024
Merged

Conversation

penelopeysm
Copy link
Contributor

@penelopeysm penelopeysm commented Oct 18, 2024

Description

Updates the QuartoNotebookRunner.jl package (used by the julia engine) to 0.11.5 to take advantage of bugfixes / updates.

Checklist

None of the below seemed relevant for such a trivial change, but please let me know if you want me to do anything else

  • filed a contributor agreement.
  • referenced the GitHub issue this PR closes
  • updated the appropriate changelog

@cscheid
Copy link
Collaborator

cscheid commented Oct 18, 2024

(Thanks!!) I don't think we need a contributor agreement for this small PR, but before we merge, I'd like to make sure we have a changelog entry that references the open bugs we have in our issue tracker.

@penelopeysm
Copy link
Contributor Author

penelopeysm commented Oct 18, 2024

Thanks @cscheid!

I think this mainly fixes one bug which I had reported upstream PumasAI/QuartoNotebookRunner.jl#192 but I didn't report on the Quarto repository. I could open / close an issue just to make a note in the changelog, if you would like that?

(I checked other bugs like #11013 and it doesn't quite work with Julia 1.11 yet)

@cscheid
Copy link
Collaborator

cscheid commented Oct 22, 2024

Hm, that's not what @MichaelHatherly claims

Can we confirm this one way or another?

@MichaelHatherly
Copy link
Contributor

Can we confirm this one way or another?

Any bug fixes listed in https://github.com/PumasAI/QuartoNotebookRunner.jl/releases from 0.11.3 to 0.11.5 will be resolved by this PR. By the looks of the issue tracker in this repo this PR will only close #11013, everything else was only tracked in QNR itself.

@cscheid
Copy link
Collaborator

cscheid commented Oct 22, 2024

Ok, thanks. Let me wait to get confirmation from @penelopeysm because she claimed 0.11.5 didn't fix #11013 for her.

@penelopeysm
Copy link
Contributor Author

I'll try again!

@penelopeysm
Copy link
Contributor Author

penelopeysm commented Oct 22, 2024

Yes, Julia 1.11 still errors for me:

index.qmd
---
title: "Home"
engine: julia
---

Hello!

```{julia}
versioninfo()
```

```{julia}
1+1
```

```{julia}
using Random
Random.seed!(1)
```

```{julia}
randn()
```

```{julia}
Random.seed!(1)
```

```{julia}
randn()
```
Error message
$ QUARTO_JULIA=~/.julia/juliaup/julia-1.11.1+0.aarch64.apple.darwin14/bin/julia quarto render index.qmd
Check file:///Users/pyong/test/quarto-cli/src/quarto.ts
Starting julia control server process. This might take a while...
Julia server process started.
ERROR: Julia server returned error after receiving "run" command:
Failed to run notebook: /Users/pyong/test/q/index.qmd
ERROR: Remote exception from Malt.Worker on port 9715 with PID 17715:

Failed to precompile QuartoNotebookWorker [38328d9c-a911-4051-bc06-3f7f556ffeda] to "/Users/pyong/.julia/compiled/v1.11/QuartoNotebookWorker/jl_ZXMqgC".
Stacktrace:
  [1] error(s::String)
    @ Base ./error.jl:35
  [2] compilecache(pkg::Base.PkgId, path::String, internal_stderr::IO, internal_stdout::IO, keep_loaded_modules::Bool; flags::Cmd, cacheflags::Base.CacheFlags, reasons::Dict{String, Int64}, isext::Bool)
    @ Base ./loading.jl:3085
  [3] (::Base.var"#1082#1083"{Base.PkgId})()
    @ Base ./loading.jl:2492
  [4] mkpidlock(f::Base.var"#1082#1083"{Base.PkgId}, at::String, pid::Int32; kwopts::@Kwargs{stale_age::Int64, wait::Bool})
    @ FileWatching.Pidfile ~/.julia/juliaup/julia-1.11.1+0.aarch64.apple.darwin14/share/julia/stdlib/v1.11/FileWatching/src/pidfile.jl:95
  [5] #mkpidlock#6
    @ ~/.julia/juliaup/julia-1.11.1+0.aarch64.apple.darwin14/share/julia/stdlib/v1.11/FileWatching/src/pidfile.jl:90 [inlined]
  [6] trymkpidlock(::Function, ::Vararg{Any}; kwargs::@Kwargs{stale_age::Int64})
    @ FileWatching.Pidfile ~/.julia/juliaup/julia-1.11.1+0.aarch64.apple.darwin14/share/julia/stdlib/v1.11/FileWatching/src/pidfile.jl:116
  [7] #invokelatest#2
    @ ./essentials.jl:1057 [inlined]
  [8] invokelatest
    @ ./essentials.jl:1052 [inlined]
  [9] maybe_cachefile_lock(f::Base.var"#1082#1083"{Base.PkgId}, pkg::Base.PkgId, srcpath::String; stale_age::Int64)
    @ Base ./loading.jl:3609
 [10] maybe_cachefile_lock
    @ ./loading.jl:3606 [inlined]
 [11] _require(pkg::Base.PkgId, env::Nothing)
    @ Base ./loading.jl:2488
 [12] __require_prelocked(uuidkey::Base.PkgId, env::Nothing)
    @ Base ./loading.jl:2315
 [13] #invoke_in_world#3
    @ ./essentials.jl:1089 [inlined]
 [14] invoke_in_world
    @ ./essentials.jl:1086 [inlined]
 [15] _require_prelocked
    @ ./loading.jl:2302 [inlined]
 [16] _require_prelocked
    @ ./loading.jl:2301 [inlined]
 [17] macro expansion
    @ ./lock.jl:273 [inlined]
 [18] require(uuidkey::Base.PkgId)
    @ Base ./loading.jl:2298
 [19] #8
    @ ~/.julia/packages/QuartoNotebookRunner/fGeE0/src/worker.jl:21 [inlined]
 [20] task_local_storage(body::var"#8#11", key::Symbol, val::Dict{String, Any})
    @ Base ./task.jl:315
 [21] top-level scope
    @ ~/.julia/packages/QuartoNotebookRunner/fGeE0/src/worker.jl:20
 [22] eval
    @ ./boot.jl:430 [inlined]
 [23] (::var"#1#2"{Sockets.TCPSocket, UInt64, Bool, @Kwargs{}, Tuple{Module, Expr}, typeof(Core.eval)})()
    @ Main ~/.julia/packages/Malt/YJ2Ml/src/worker.jl:120
Stacktrace:
  [1] unwrap_worker_result(worker::Malt.Worker, result::Malt.WorkerResult)
    @ Malt ~/.julia/packages/Malt/YJ2Ml/src/Malt.jl:50
  [2] _wait_for_response(worker::Malt.Worker, msg_id::UInt64)
    @ Malt ~/.julia/packages/Malt/YJ2Ml/src/Malt.jl:325
  [3] _send_receive
    @ ~/.julia/packages/Malt/YJ2Ml/src/Malt.jl:336 [inlined]
  [4] #remote_call_fetch#41
    @ ~/.julia/packages/Malt/YJ2Ml/src/Malt.jl:406 [inlined]
  [5] remote_call_fetch
    @ ~/.julia/packages/Malt/YJ2Ml/src/Malt.jl:405 [inlined]
  [6] remote_eval_fetch
    @ ~/.julia/packages/Malt/YJ2Ml/src/Malt.jl:484 [inlined]
  [7] remote_eval_fetch
    @ ~/.julia/packages/Malt/YJ2Ml/src/Malt.jl:485 [inlined]
  [8] init!
    @ ~/.julia/packages/QuartoNotebookRunner/fGeE0/src/server.jl:105 [inlined]
  [9] QuartoNotebookRunner.File(path::String, options::Dict{String, Any})
    @ QuartoNotebookRunner ~/.julia/packages/QuartoNotebookRunner/fGeE0/src/server.jl:26
 [10] (::QuartoNotebookRunner.var"#41#44"{Bool, Dict{String, Any}, Server, String})()
    @ QuartoNotebookRunner ~/.julia/packages/QuartoNotebookRunner/fGeE0/src/server.jl:1149
 [11] lock(f::QuartoNotebookRunner.var"#41#44"{Bool, Dict{String, Any}, Server, String}, l::ReentrantLock)
    @ Base ./lock.jl:232
 [12] borrow_file!(f::QuartoNotebookRunner.var"#32#36"{Nothing, String, Bool, Dict{String, Any}, QuartoNotebookRunner.var"#chunk_callback#67"{TCPSocket}, Server}, server::Server, path::String; optionally_create::Bool, options::Dict{String, Any})
    @ QuartoNotebookRunner ~/.julia/packages/QuartoNotebookRunner/fGeE0/src/server.jl:1142
 [13] borrow_file!
    @ ~/.julia/packages/QuartoNotebookRunner/fGeE0/src/server.jl:1133 [inlined]
 [14] #run!#31
    @ ~/.julia/packages/QuartoNotebookRunner/fGeE0/src/server.jl:1102 [inlined]
 [15] _handle_response(socket::TCPSocket, notebooks::Server, request::@NamedTuple{type::String, content::Union{Dict{String, Any}, String}}, showprogress::Bool)
    @ QuartoNotebookRunner ~/.julia/packages/QuartoNotebookRunner/fGeE0/src/socket.jl:273
 [16] (::QuartoNotebookRunner.var"#59#66"{Bool, Base.RefValue{Bool}, Server, Base.UUID})()
    @ QuartoNotebookRunner ~/.julia/packages/QuartoNotebookRunner/fGeE0/src/socket.jl:201
ERROR: Internal julia server error

Stack trace:
    at writeJuliaCommand (file:///Users/pyong/test/quarto-cli/src/execute/julia.ts:701:13)
    at eventLoopTick (ext:core/01_core.js:175:7)
    at async executeJulia (file:///Users/pyong/test/quarto-cli/src/execute/julia.ts:529:20)
    at async Object.execute (file:///Users/pyong/test/quarto-cli/src/execute/julia.ts:141:16)
    at async renderExecute (file:///Users/pyong/test/quarto-cli/src/command/render/render-files.ts:232:25)
    at async renderFileInternal (file:///Users/pyong/test/quarto-cli/src/command/render/render-files.ts:592:35)
    at async renderFiles (file:///Users/pyong/test/quarto-cli/src/command/render/render-files.ts:325:9)
    at async renderProject (file:///Users/pyong/test/quarto-cli/src/command/render/project.ts:457:23)
    at async Command.actionHandler (file:///Users/pyong/test/quarto-cli/src/command/render/cmd.ts:248:26)
    at async Command.execute (https://deno.land/x/[email protected]/command/command.ts:1948:7)
Version of QuartoNotebookRunner
$ grep 'version' ~/.julia/packages/QuartoNotebookRunner/fGeE0/Project.toml
version = "0.11.5"

I'm using the version of Quarto on the main branch, with the change in this PR.

I'm not sure why this specifically happens with Quarto, as QuartoNotebookRunner.jl precompiles totally fine when I add it to a new Julia environment.

@MichaelHatherly
Copy link
Contributor

@penelopeysm please open an issue in the runner repo with those details and we'll attempt to recreate it.

#11013 was specific to the InitError: ConcurrencyViolationError("deadlock detected in loading QuartoNotebookWorker -> QuartoNotebookWorker") issue. We've not seen the specific stacktrace above before.

@penelopeysm
Copy link
Contributor Author

@MichaelHatherly Thanks, will do :)

@penelopeysm
Copy link
Contributor Author

In the process of attempting to reproduce the above, it now works 😳

I wonder if there was some caching going on in my system? Getting rid of the entirety of ~.julia and ~/Library/Caches/quarto before trying to repro with fresh installs seemed to be what fixed it.

Sorry to waste your time!

@penelopeysm
Copy link
Contributor Author

Didn't mean to close, sorry. But changelog entry added now.

@penelopeysm penelopeysm reopened this Oct 22, 2024
@MichaelHatherly
Copy link
Contributor

No worries. If it shows up again just open an issue and we'll take a look.

@cscheid
Copy link
Collaborator

cscheid commented Oct 22, 2024

I wonder if there was some caching going on in my system? Getting rid of the entirety of ~.julia and ~/Library/Caches/quarto before trying to repro with fresh installs seemed to be what fixed it.

Glad to hear it! We'll add a note to https://quarto.org/docs/troubleshooting/index.html in case it helps other people.

@jkrumbiegel
Copy link
Contributor

jkrumbiegel commented Oct 22, 2024

Until quarto gets some kind of convencience commands for julia, the only option you have to make sure that you start with a fresh server process is to find the pid of the current one and kill it. You can do that either by finding the right julia process with tools like ps (you'd see that this process executed a julia file that is stored in quarto's directory) or by doing quarto render somefile.qmd --execute-debug and you'll see the pid that it tries to connect to there.

Only after that, things like the QUARTO_JULIA_PROJECT env var will have an effect.

@cscheid
Copy link
Collaborator

cscheid commented Oct 22, 2024

Until quarto gets some kind of convencience commands for julia

Can we help you with that? I'd love for Quarto to be able to assist better.

@jkrumbiegel
Copy link
Contributor

I was thinking about a couple things. We might want to switch to a system in the future where we don't pin the exact QuartoNotebookRunner version in quarto anymore (because then we need a new quarto release for every patch update). I did this as the conservative option to start with. But if there was a quarto julia check and quarto julia update then one could update painlessly if there's a new version. Another could be quarto julia restart to kill and restart the server process (in case its gotten into a bad state).

@cscheid
Copy link
Collaborator

cscheid commented Oct 22, 2024

Cool. Would quarto engine julia check be too verbose?

I'm wondering about allowing engines to register arbitrary commands. We have a similar problem with the jupyter kernel daemon often needing restarting, and quarto engine jupyter check would also be useful.

@jkrumbiegel
Copy link
Contributor

I don't think that's too verbose at all if it makes namespacing such commands easier. I like a good namespace 😄

@cscheid
Copy link
Collaborator

cscheid commented Oct 22, 2024

Great - I'm tracking that here: #11159

@cscheid cscheid merged commit 2406561 into quarto-dev:main Oct 23, 2024
52 of 55 checks passed
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.

4 participants