Skip to content

Forward envvars from JSON payload #306

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Forward envvars from JSON payload #306

wants to merge 1 commit into from

Conversation

MichaelHatherly
Copy link
Collaborator

@MichaelHatherly MichaelHatherly commented May 22, 2025

This linked PR is changing the way that several QUARTO_* environment variables are forwarded to the engine. Instead of just being available via ENV they are passed in a .env field in the options JSON that Quarto passes to QNR. Pick out the env vars if they exist and add them to the notebook process's env vars.

Cc @cscheid, this will provide the fix needed for quarto-dev/quarto-cli#12621.

@MichaelHatherly
Copy link
Collaborator Author

MichaelHatherly commented May 22, 2025

@cscheid if you're wanting to run this in CI in quarto-dev/quarto-cli#12621 then temporarily committing a Manifest.toml to https://github.com/quarto-dev/quarto-cli/tree/main/src/resources/julia that points to this branch should do the trick. (Not sure how familiar you are with that part of Julia so let me know if unsure.)

Copy link

codecov bot commented May 22, 2025

Codecov Report

Attention: Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 79.42%. Comparing base (58e5dc8) to head (fb14ffa).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/server.jl 91.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #306      +/-   ##
==========================================
+ Coverage   79.40%   79.42%   +0.01%     
==========================================
  Files          39       39              
  Lines        2064     2066       +2     
==========================================
+ Hits         1639     1641       +2     
  Misses        425      425              

☔ 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.

@cscheid
Copy link

cscheid commented May 22, 2025

@cscheid if you're wanting to run this in CI in quarto-dev/quarto-cli#12621 then temporarily committing a Manifest.toml to https://github.com/quarto-dev/quarto-cli/tree/main/src/resources/julia that points to this branch should do the trick. (Not sure how familiar you are with that part of Julia so let me know if unsure.)

Thanks, I will take you on your offer. I'm trying to do this locally and failing. I've confirmed that the branch I'm working on fails without this version of QuartoNotebookRunner:

 quarto preview julia.qmd
Starting julia control server process. This might take a while...
Julia server process started.
Running [1/1] at line 12:  ENV["QUARTO_DOCUMENT_FILE"]
ERROR: Julia server returned error after receiving "run" command:

Failed to run notebook: /Users/cscheid/repos/github/quarto-dev/quarto-cli/tests/docs/smoke-all/engine/env/julia.qmd

The underlying Julia error was:

EvaluationError: Encountered 1 error during evaluation

Error 1 of 1
@ /Users/cscheid/repos/github/quarto-dev/quarto-cli/tests/docs/smoke-all/engine/env/julia.qmd:12
KeyError: key "QUARTO_DOCUMENT_FILE" not found
Stacktrace:
 [1] (::Base.var"#693#694")(k::String)
   @ Base ./env.jl:100
 [2] access_env
   @ ./env.jl:43 [inlined]
 [3] getindex(#unused#::Base.EnvDict, k::String)
   @ Base ./env.jl:100
 [4] top-level scope
   @ ~/repos/github/quarto-dev/quarto-cli/tests/docs/smoke-all/engine/env/julia.qmd:13

However, adding the following Manifest.toml file to the .../engine/env directory doesn't appear to do anything:

[[QuartoNotebookRunner]]
git-tree-sha1 = "57f9bd985428b9112fa258a7d889eb8cc90db19d"
uuid = "4c0109c6-14e9-4c88-93f0-2b974d3468f4"
version = "0.17.3"
repo-rev = "mh/env"
repo-url = "https://github.com/PumasAI/QuartoNotebookRunner.jl"

It's almost certain that I'm doing something wrong, but I'm not sure what that would be.

@MichaelHatherly
Copy link
Collaborator Author

However, adding the following Manifest.toml file to the .../engine/env directory doesn't appear to do anything:

engine/env/julia.qmd:13

Looks like you're installing QNR into the notebook's environment rather than the server environment.

Needs to be installed into this one: https://github.com/quarto-dev/quarto-cli/tree/main/src/resources/julia

@cscheid
Copy link

cscheid commented May 22, 2025

Ok, I think I got it to update, but I don't think the environment is being updated through correctly:

$ git log
commit 6028e0b354b7a15e908940a4c6819aad8e19e7b1 (HEAD -> bugfix/no-env-set-past-startup, origin/bugfix/no-env-set-past-startup)
...
$ quarto render julia.qmd
Starting julia control server process. This might take a while...
    Updating registry at `~/.julia/registries/General.toml`
   Installed QuartoNotebookRunner ─ v0.17.3
    Updating `~/Library/Caches/quarto/julia/Project.toml`
  [4c0109c6] ↑ QuartoNotebookRunner v0.17.0 ⇒ v0.17.3
    Updating `~/Library/Caches/quarto/julia/Manifest.toml`
  [4c0109c6] ↑ QuartoNotebookRunner v0.17.0 ⇒ v0.17.3
Precompiling project...
  1 dependency successfully precompiled in 14 seconds. 20 already precompiled.
[ Info: We haven't cleaned this depot up for a bit, running Pkg.gc()...
      Active manifest files: 6 found
      Active artifact files: 158 found
      Active scratchspaces: 4 found
     Deleted 2 package installations (639.244 KiB)
     Deleted 3 artifact installations (8.487 MiB)
Julia server process started.
Running [1/1] at line 12:  ENV["QUARTO_DOCUMENT_FILE"]
ERROR: Julia server returned error after receiving "run" command:

Failed to run notebook: /Users/cscheid/repos/github/quarto-dev/quarto-cli/tests/docs/smoke-all/engine/env/julia.qmd

The underlying Julia error was:

EvaluationError: Encountered 1 error during evaluation

Error 1 of 1
@ /Users/cscheid/repos/github/quarto-dev/quarto-cli/tests/docs/smoke-all/engine/env/julia.qmd:12
KeyError: key "QUARTO_DOCUMENT_FILE" not found
Stacktrace:
 [1] (::Base.var"#693#694")(k::String)
   @ Base ./env.jl:100
 [2] access_env
   @ ./env.jl:43 [inlined]
 [3] getindex(#unused#::Base.EnvDict, k::String)
   @ Base ./env.jl:100
 [4] top-level scope
   @ ~/repos/github/quarto-dev/quarto-cli/tests/docs/smoke-all/engine/env/julia.qmd:13

@MichaelHatherly
Copy link
Collaborator Author

Does ~/Library/Caches/quarto/Julia/Manifest.toml reference this branch, or just 0.17.3?

@cscheid
Copy link

cscheid commented May 22, 2025

[[deps.QuartoNotebookRunner]]
deps = ["BSON", "Base64", "CommonMark", "Compat", "Dates", "IOCapture", "InteractiveUtils", "IterTools", "JSON3", "Logging", "Pkg", "PrecompileTools", "Preferences", "ProgressLogging", "REPL", "Random", "RelocatableFolders", "SHA", "Sockets", "TOML", "YAML"]
git-tree-sha1 = "94a4a9ef9ee5b22b5ea01215f749114f5cf9e220"
uuid = "4c0109c6-14e9-4c88-93f0-2b974d3468f4"
version = "0.17.3"

This commit is 57f9... so I think the answer is "just 0.17.3"

@jkrumbiegel
Copy link
Collaborator

Isn't the quarto CI running a sufficiently new Julia version that a [sources] section in the Project.toml would suffice, instead of committing a Manifest?
Like this (untested)

[sources]
QuartoNotebookRunner = {url = "https://github.com/PumasAI/QuartoNotebookRunner.jl/", rev = "mh/env"}

@MichaelHatherly
Copy link
Collaborator Author

Locally you'd do something like:

julia --project=~/Library/Caches/quarto/Julia

julia> ]add QuartoNotebookRunner#mh/env

to change it.

@MichaelHatherly
Copy link
Collaborator Author

Yeah, they're on 1.11 then that'll work as well

@MichaelHatherly
Copy link
Collaborator Author

@cscheid did you get a chance to try this out to see whether it was doing what had wanted it to do?

@cscheid
Copy link

cscheid commented Jun 11, 2025

@MichaelHatherly I'm sorry for the delay. I was out of commission for the last 10 days and I'm getting back today. I'll report back here as soon as I have updates (hopefully today even.)

@cscheid
Copy link

cscheid commented Jun 16, 2025

Hi! This worked 🎉

$ julia --project=~/Library/Caches/quarto/Julia
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.9.2 (2023-07-05)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

(julia) pkg> add QuartoNotebookRunner#mh/env
     Cloning git-repo `https://github.com/PumasAI/QuartoNotebookRunner.jl.git`
    Updating git-repo `https://github.com/PumasAI/QuartoNotebookRunner.jl.git`
    Updating registry at `~/.julia/registries/General.toml`
   Resolving package versions...
    Updating `~/Library/Caches/quarto/julia/Project.toml`
  [4c0109c6] ~ QuartoNotebookRunner v0.17.3 ⇒ v0.17.3 `https://github.com/PumasAI/QuartoNotebookRunner.jl.git#mh/env`
    Updating `~/Library/Caches/quarto/julia/Manifest.toml`
  [4c0109c6] ~ QuartoNotebookRunner v0.17.3 ⇒ v0.17.3 `https://github.com/PumasAI/QuartoNotebookRunner.jl.git#mh/env`
Precompiling project...
  1 dependency successfully precompiled in 14 seconds. 20 already precompiled.

(julia) pkg>
$ quarto render julia.qmd
Check file:///Users/cscheid/repos/github/quarto-dev/quarto-cli/src/quarto.ts
Starting julia control server process. This might take a while...
    Updating registry at `~/.julia/registries/General.toml`
    Updating git-repo `https://github.com/PumasAI/QuartoNotebookRunner.jl.git`
   Installed JSON3 ─ v1.14.3
   Installed YAML ── v0.4.14
  No Changes to `~/Library/Caches/quarto/julia/Project.toml`
    Updating `~/Library/Caches/quarto/julia/Manifest.toml`
  [0f8b85d8] ↑ JSON3 v1.14.2 ⇒ v1.14.3
  [ddb6d928] ↑ YAML v0.4.13 ⇒ v0.4.14
Precompiling project...
  3 dependencies successfully precompiled in 21 seconds. 18 already precompiled.
[ Info: We haven't cleaned this depot up for a bit, running Pkg.gc()...
      Active manifest files: 6 found
      Active artifact files: 158 found
      Active scratchspaces: 4 found
     Deleted 1 package installation (483.699 KiB)
     Deleted 9 artifact installations (130.005 MiB)
     Deleted 2 scratchspaces (111.047 KiB)
Julia server process started.
Running [1/1] at line 6:  print("hi")
pandoc
  to: html
  output-file: julia.html
  standalone: true
  section-divs: true
  html-math-method: mathjax
  wrap: none
  default-image-extension: png
  variables: {}

metadata
  document-css: false
  link-citations: true
  date-format: long
  lang: en

Output created: julia.html

@MichaelHatherly MichaelHatherly marked this pull request as ready for review June 20, 2025 14:16
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.

3 participants