[pyoutline/rqd] Add pycuerun entry point, fix CLI bugs, and improve sandbox support#2215
Merged
ramonfigueiredo merged 11 commits intoAcademySoftwareFoundation:masterfrom Apr 2, 2026
Conversation
- Add quick start guides for PyOutline and PyCuerun - Add concept pages explaining core architecture, lifecycle, and design - Add getting started guides for installation and first job submission - Add user guides covering layers, dependencies, sessions, and CLI usage - Add configuration guide for outline.cfg, plugins, and environment variables - Add sessions and data exchange guide for cross-layer communication - Add PyOutline API reference for all classes, methods, and constants - Add step-by-step tutorials (PyOutline and PyCuerun lessons) - Add developer guides covering architecture, testing, and extension points - Add PyCuerun entry to the glossary - Document the relationship between PyOutline (library) and PyCuerun (CLI) across all new docs
…andbox support - Add pycuerun as a proper pip-installable entry point via pyproject.toml - Fix missing --priority option in CuerunOptionParser - Fix empty --repos argument consuming the next CLI flag - Fix use_pycuerun=False crash (TypeError) for layers without a command arg - Add bash-based opencue_wrap_frame_sandbox wrapper for environments without tcsh - Build rqd Docker image from source with pyoutline and pycue installed - Update docker-compose.yml to build rqd from source with volume mounts for frame execution
- Remove '--repos ' from testSerializeShellOutline expected command string - Remove '--repos ' from testBuildShellCommand expected list - Remove '--repos ' from testBuildCommandWithStrace expected list - Remove '--repos ' from testBuildCommandWithCustomWrapper expected list
Collaborator
Author
|
This PR was created on top of the PR: |
ramonfigueiredo
force-pushed
the
fix/pyoutline-pycuerun-sandbox
branch
from
906bf06 to
0140559
Compare
- Create outline config override with session_dir under /tmp - The default ~/.opencue/sessions is owned by root on CI runners - Export OUTLINE_CONFIG_FILE so pyoutline uses the writable path
ramonfigueiredo
force-pushed
the
fix/pyoutline-pycuerun-sandbox
branch
from
0140559 to
7ce857f
Compare
- Previous /tmp/opencue path was also owned by root on CI runners - mktemp guarantees a writable directory owned by the current user
Collaborator
Author
|
@DiegoTavares / @lithorus |
lithorus
reviewed
Mar 16, 2026
Collaborator
|
What's the reason for IMO it shouldn't be necessary to build rqd from source to run rqd in docker compose. It might be usefull for development, but not for testing for new users. It also changes a lot in the CI/CD testing. The idea being that packages are built in a seperate "neutral" step and used in the subsequent steps. |
DiegoTavares
approved these changes
Mar 23, 2026
lithorus
reviewed
Mar 25, 2026
Collaborator
Author
Hi @lithorus I recalled having some issues with the current version of the Python RQD. FYI... there is a new PR from @DiegoTavares that updates the docker-compose.yml to use the new Rust RQD. Since Diego has already approved my PR, I am going to merge the PR now. We can submit further PRs later if needed. |
- The previous error message was misleading. It complained about use_pycuerun not being enabled when the actual issue is a missing command. - The new message clearly states the problem and offers both options to resolve it.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCreates per-run Outline session/config in CI, switches rqd to a local Docker build with session mounts, replaces wheel installs with source-based pip installs in the rqd image, adds pycuerun console entrypoint and docs, tightens Cue backend serialization and CLI (adds --priority), introduces a sandbox frame wrapper, and updates tests/docs accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer/CI
participant Cuebot as Cuebot
participant RQD as RQD (container)
participant Wrapper as opencue_wrap_frame_sandbox
participant PyCuerun as pycuerun
Dev->>Cuebot: Submit Outline (OUTLINE_CONFIG_FILE with session_dir)
Cuebot->>RQD: Dispatch frame job
RQD->>Wrapper: Start sandbox wrapper with workdir + command
Wrapper->>PyCuerun: Invoke pycuerun -e for frame
PyCuerun-->>Wrapper: Exit status
Wrapper-->>RQD: Cleanup and return status
RQD-->>Cuebot: Report frame result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
Good point, @lithorus . The reason |
Collaborator
Author
|
For more information, see the documentation: |
Collaborator
I understand. I might have a look at it at a later point. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (1)
pyoutline/tests/backend/test_cue.py (1)
149-157: Add a positive test case for non-emptyrepos.These updates validate omission of
--repos, but the new conditional path should also be covered when repos is set to ensure no regression on emission behavior.Suggested additional test
class BuildCommandTest(unittest.TestCase): @@ def testBuildShellCommand(self): self.assertEqual( @@ outline.backend.cue.build_command(self.launcher, self.layer)) + + `@mock.patch`("outline.versions.get_repos", return_value="/show/repos") + def testBuildShellCommandWithRepos(self, _repos_mock): + cmd = outline.backend.cue.build_command(self.launcher, self.layer) + self.assertIn("--repos /show/repos", cmd)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyoutline/tests/backend/test_cue.py` around lines 149 - 157, Add a new positive unit test that covers the non-empty repos path: create a test (e.g., testBuildShellCommandWithRepos) that sets self.layer.repos to a non-empty list (or sets repos on the same object used by build_command), call outline.backend.cue.build_command(self.launcher, self.layer) and assert the returned command contains the --repos flag with the repos joined in the expected format (and still contains the other existing pieces like '/wrappers/opencue_wrap_frame', '/bin/pycuerun', the shell outline invocation, '--version latest', '--debug'); this ensures build_command handles emission of --repos when repos is present without regressing the rest of the command.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ci/run_integration_test.sh`:
- Around line 185-190: SESSION_DIR is created with mktemp -d in a location not
bind-mounted into the render container; change creation to place the temp
directory under a bind-mounted root (e.g. create SESSION_DIR with mktemp -d
"${HOME}/.opencue/sessions/session.XXXX" or under /tmp/opencue) so the render
container can read it, and split the creation and export into two statements so
mktemp failures surface under set -e; keep OUTLINE_CONFIG_FILE creation/export
the same but ensure session_dir uses the new SESSION_DIR variable when writing
the config.
In `@docker-compose.yml`:
- Around line 205-209: The session mounts in docker-compose.yml are marked
read-only which will break runtime writes from frame code (see
put_data()/put_file() called in _execute()); make the session root mounts
writable while keeping any user_dir mounts read-only: change the bindings for
the session/session_dir and /tmp/opencue mounts to be writable (remove :ro) so
sandboxed frames can write back data, and if user_dir must remain immutable, add
a separate bind for that specific path with :ro rather than marking the entire
session tree read-only.
In `@docs/_docs/concepts/pycuerun-concepts.md`:
- Around line 63-68: The fenced code blocks showing the diagram starting with
"pycuerun my_script.outline" (and the similar arrow diagrams) are missing a
language tag and trigger MD040; update each fenced block (the ones containing
the lines with "pycuerun my_script.outline" and the ASCII arrow diagram) to
include a language tag such as ```text so the blocks become fenced with ```text
and avoid markdownlint failures.
In `@docs/_docs/concepts/pyoutline-concepts.md`:
- Around line 74-80: The markdown fenced code blocks containing the
diagram/snippet (e.g., the block starting with Layer "render" with range 1-10
and the block with INIT --> SETUP --> READY) are missing language
identifiers; update each opening ``` to ```text so the snippets become fenced as
text (for example change ``` to ```text for the "Layer \"render\" ..." block and
the "INIT --> SETUP --> READY" block) to satisfy markdownlint MD040 and
CI/docs checks.
In `@docs/_docs/developer-guide/pycuerun-development.md`:
- Around line 63-64: Update the docs to stop referencing a non-existent concrete
class "PyCuerun" and instead use the actual symbol names found in the codebase:
replace occurrences of "PyCuerun" with "AbstractCuerun" (or clearly state that
bin/pycuerun provides an entrypoint wrapping AbstractCuerun) and adjust the
class hierarchy diagrams and prose in the sections noted (lines ~63, ~101-102,
~127-130) to reference AbstractCuerun and the module path
(pyoutline/bin/cuerunbase.py) so the documentation matches the real API and
extension points.
- Around line 58-96: The fenced diagram/code blocks in the developer guide are
missing language tags; for each diagram containing items like "pycuerun
[options] script.outline", "PyCuerun (AbstractCuerun)", function names
convert_sys_args_from_olrun, handle_core_args, CuerunOptionParser,
handle_my_options and downstream labels execute_frame, inspect_script,
launch_outline, update the opening triple-backtick to include a language
identifier (use text) so the fences become ```text to satisfy MD040 across those
blocks.
In `@docs/_docs/developer-guide/pyoutline-development.md`:
- Around line 303-336: The doc's backend interface is incorrect about a uniform
build_command signature; update the text to state that build_command signatures
vary by backend and enumerate the actual signatures used (e.g., cue backend's
build_command(launcher, layer) and local backend's build_command(ol, layer,
frame)), or alternatively provide per-backend contract sections describing
required function names and exact parameters for launch(launcher,
use_pycuerun=True), serialize(launcher) and build_command(...). Mention the
backends that deviate (cue.py and local.py) and ensure the docs indicate callers
should consult the specific backend implementation for exact parameter lists.
- Around line 96-112: Update the docs to reflect the real LayerType behavior:
change the text and sample to show that LayerType.__call__ auto-registers only
when layer.get_arg("register") is truthy (instead of always registering), that
it calls PluginManager.init_plugin_all(layer) to run plugin init hooks, and that
it does NOT fire AFTER_INIT events; remove the claim about firing AFTER_INIT and
adjust the example to include the conditional register check and plugin init
call (referencing LayerType.__call__, current_outline(), outline.add_layer,
PluginManager.init_plugin_all, layer.get_arg("register"), and AFTER_INIT).
In `@docs/_docs/getting-started/getting-started-pyoutline.md`:
- Around line 70-75: Docs list omits the deprecated environment variable
OL_CONFIG which is still checked by pyoutline; update the precedence text to
match pyoutline/outline/config.py by inserting OL_CONFIG between
OUTLINE_CONFIG_FILE and the user config path. Specifically, change the ordered
list to: 1) path from OUTLINE_CONFIG_FILE, 2) path from OL_CONFIG (deprecated),
3) ~/.config/opencue/outline.cfg (user profile), 4) built-in defaults so the
documentation reflects the actual lookup in config.py.
In `@docs/_docs/quick-starts/quick-start-pycuerun.md`:
- Around line 23-26: The docs state "Python 3.7 or later" but package metadata
in pycue/pyproject.toml, pyoutline/pyproject.toml, and rqd/pyproject.toml
require Python >3.7 (i.e. 3.8+); update the quick-start text in
docs/_docs/quick-starts/quick-start-pycuerun.md to read "Python 3.8 or later"
(or "Python 3.8+") and ensure any other user-facing docs that mention supported
Python versions match the requires-python constraints in pycue/pyproject.toml,
pyoutline/pyproject.toml, and rqd/pyproject.toml so the messaging is consistent.
In `@docs/_docs/reference/pyoutline-api-reference.md`:
- Around line 171-173: Documentation for put_data omits the force parameter;
update the API reference entries for put_data (both the generic `put_data(key,
data)` row and any other occurrences like the later `put_data` listing) to
include the `force` argument and a short description (e.g., `put_data(key, data,
force=False)` — "Store data in the session; set force=True to overwrite existing
data"). Ensure this change documents both outline.Layer.put_data and
outline.Session.put_data so the reference matches the implementations.
- Around line 218-223: The documented constructor signature for LayerPreProcess
is wrong — change the example from LayerPreProcess(name, **args) to
LayerPreProcess(creator, **args) and update the description to explain that
`creator` is the parent layer, the preprocess layer is auto-named
`{creator_name}_preprocess` (or uses a custom `suffix`), is marked as a utility
layer and depends to run before the parent unlocks, and that outputs stored via
`put_data()` on the preprocess layer are available to the parent layer;
reference LayerPreProcess, creator, suffix, and put_data in the updated text.
In `@docs/_docs/tutorials/pyoutline-tutorial.md`:
- Line 231: The heading string "## Tutorial 6: Pre and Post Processing" should
be updated to use a hyphenated compound modifier; replace it with "## Tutorial
6: Pre- and Post-Processing" so the heading in the file (the heading line that
currently reads "Tutorial 6: Pre and Post Processing") follows correct
compound-modifier style and matches other technical headings.
In `@docs/_docs/user-guides/pycuerun-user-guide.md`:
- Around line 153-159: The docs currently document CLI flags (-i/--inspect and
--qc) that are not implemented in the CuerunOptionParser class; update the user
guide to either remove the Inspect and QC command examples or explicitly mark
them as plugin-specific and list prerequisites and where they come from (e.g.,
which plugin or entrypoint provides those flags), and do the same for the other
affected sections referenced in your review; search for CuerunOptionParser and
the Inspect/QC examples to locate the exact spots to edit and ensure the guide
points readers to the plugin name or installation/enablement steps required to
use those flags.
- Around line 163-172: The docs show a combined single-token syntax (`-e
5-render`) but the CLI actually parses frame and layer as separate arguments for
execute_frame(script, layer, frame); update the examples and explanatory text to
reflect the real invocation (pass frame and layer separately or the actual flag
parsing used by the CLI), e.g., show the correct CLI form that maps to
execute_frame(script, layer, frame) and change the `{frame_number}-{layer_name}`
description to the correct parameter format; also apply the same fix to the
other occurrence mentioned.
In `@docs/_docs/user-guides/pyoutline-user-guide.md`:
- Around line 337-339: The documented configuration search order omitted the
deprecated OL_CONFIG fallback; update the list that currently shows
OUTLINE_CONFIG_FILE, ~/.config/opencue/outline.cfg, and Built-in defaults to
include OL_CONFIG (marked deprecated) in the correct resolution order so the
guide matches the configuration behavior and the PR's troubleshooting guidance.
In `@pyoutline/outline/backend/cue.py`:
- Around line 95-97: The repos value returned by outline.versions.get_repos()
can be whitespace-only and currently passes the truthiness check, producing a
malformed "--repos" argument; update the code that builds the command (the block
referencing repos and appending to command) to trim/repos = repos.strip() (or
equivalent) before checking truthiness and only append "--repos %s" when the
trimmed repos is non-empty so whitespace-only strings are ignored.
---
Nitpick comments:
In `@pyoutline/tests/backend/test_cue.py`:
- Around line 149-157: Add a new positive unit test that covers the non-empty
repos path: create a test (e.g., testBuildShellCommandWithRepos) that sets
self.layer.repos to a non-empty list (or sets repos on the same object used by
build_command), call outline.backend.cue.build_command(self.launcher,
self.layer) and assert the returned command contains the --repos flag with the
repos joined in the expected format (and still contains the other existing
pieces like '/wrappers/opencue_wrap_frame', '/bin/pycuerun', the shell outline
invocation, '--version latest', '--debug'); this ensures build_command handles
emission of --repos when repos is present without regressing the rest of the
command.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 26b1100d-699a-468a-b169-af3cc183e0da
📒 Files selected for processing (26)
ci/run_integration_test.shdocker-compose.ymldocs/_docs/concepts/glossary.mddocs/_docs/concepts/pycuerun-concepts.mddocs/_docs/concepts/pyoutline-concepts.mddocs/_docs/developer-guide/pycuerun-development.mddocs/_docs/developer-guide/pyoutline-development.mddocs/_docs/getting-started/getting-started-pycuerun.mddocs/_docs/getting-started/getting-started-pyoutline.mddocs/_docs/other-guides/pyoutline-configuration.mddocs/_docs/other-guides/pyoutline-sessions-and-data.mddocs/_docs/quick-starts/quick-start-pycuerun.mddocs/_docs/quick-starts/quick-start-pyoutline.mddocs/_docs/reference/pyoutline-api-reference.mddocs/_docs/tutorials/pycuerun-tutorial.mddocs/_docs/tutorials/pyoutline-tutorial.mddocs/_docs/user-guides/pycuerun-user-guide.mddocs/_docs/user-guides/pyoutline-user-guide.mdpyoutline/bin/__init__.pypyoutline/bin/pycuerun_main.pypyoutline/outline/backend/cue.pypyoutline/outline/cuerun.pypyoutline/pyproject.tomlpyoutline/tests/backend/test_cue.pypyoutline/wrappers/opencue_wrap_frame_sandboxrqd/Dockerfile
- Place CI session_dir under /tmp/opencue so it is accessible inside the rqd container via the existing bind mount - Split OUTLINE_CONFIG_FILE assignment and export so mktemp failures surface under set -e - Make session bind mounts writable in docker-compose.yml so frames can write back data via put_data()/put_file() - Add 'text' language tag to bare fenced code blocks in docs to satisfy markdownlint MD040
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (8)
docs/_docs/concepts/pycuerun-concepts.md (3)
63-68:⚠️ Potential issue | 🟡 MinorAdd language tag to fenced code block (duplicate issue).
This fenced block is missing a language identifier and triggers MD040. Add
textafter the opening fence.📝 Proposed fix
-``` +```text pycuerun my_script.outline | v🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/_docs/concepts/pycuerun-concepts.md` around lines 63 - 68, The fenced code block containing the outline pipeline diagram (starting with the line "pycuerun my_script.outline" and the following arrow lines) is missing a language tag and triggers MD040; fix it by adding the language identifier `text` immediately after the opening triple backticks of that fenced block so the block becomes a ```text fenced code block.
87-89:⚠️ Potential issue | 🟡 MinorAdd language tag to fenced code block (duplicate issue).
This fenced block is missing a language identifier and triggers MD040. Add
textafter the opening fence.📝 Proposed fix
-``` +```text opencue_wrap_frame --> pycuerun -e {frame}-{layer} script.outline🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/_docs/concepts/pycuerun-concepts.md` around lines 87 - 89, The fenced code block containing "opencue_wrap_frame --> pycuerun -e {frame}-{layer} script.outline" is missing a language identifier and triggers MD040; update the opening fence to include the language tag "text" (i.e., change the opening triple backticks to "```text") so the block becomes a labeled text code block and satisfies the linter.
74-79:⚠️ Potential issue | 🟡 MinorAdd language tag to fenced code block (duplicate issue).
This fenced block is missing a language identifier and triggers MD040. Add
textafter the opening fence.📝 Proposed fix
-``` +```text pycuerun -e 5-render my_script.outline | v🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/_docs/concepts/pycuerun-concepts.md` around lines 74 - 79, The fenced code block showing the pycuerun example is missing a language identifier and triggers MD040; update the opening fence for that block (the triple-backtick that precedes the lines "pycuerun -e 5-render my_script.outline" and the arrow lines) to include the language tag "text" so it becomes ```text, leaving the block contents unchanged.docs/_docs/developer-guide/pycuerun-development.md (5)
396-398:⚠️ Potential issue | 🟡 MinorAdd language tag to fenced code block (duplicate issue).
This fenced block showing the command structure is missing a language identifier. Add
textafter the opening fence.📝 Proposed fix
-``` +```text [strace ...] <wrapper> <user_dir> <pycuerun> <script> -e `#IFRAME`#-<layer> --version <ver> --repos <repos> --debug [--dev] [--dev-user <user>]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/_docs/developer-guide/pycuerun-development.md` around lines 396 - 398, The fenced code block showing the command structure is missing a language tag; update the opening fence from ``` to ```text so it reads ```text followed by the command line "[strace ...] <wrapper> <user_dir> <pycuerun> <script> -e `#IFRAME`#-<layer> --version <ver> --repos <repos> --debug [--dev] [--dev-user <user>]" to ensure the block is annotated as plain text; modify the block that currently begins with ``` and the matching closing ``` to use the proposed ```text opening as shown in the diff.
170-210:⚠️ Potential issue | 🟡 MinorAdd language tag to fenced code block (duplicate issue).
This fenced block showing code organization is missing a language identifier. Add
textafter the opening fence.📝 Proposed fix
-``` +```text pyoutline/ ├── bin/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/_docs/developer-guide/pycuerun-development.md` around lines 170 - 210, The code block showing the project tree (the fenced block that begins with the line "pyoutline/") is missing a language identifier; update the opening fence so it reads ```text (i.e., add the 'text' language tag) for the fenced block containing the "pyoutline/" tree to ensure proper syntax highlighting.
100-141:⚠️ Potential issue | 🟡 MinorAdd language tag to fenced code block (duplicate issue).
This fenced block containing the component relationships diagram is missing a language identifier. Add
textafter the opening fence.📝 Proposed fix
-``` +```text bin/pycuerun CLI entry point, PyCuerun class │🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/_docs/developer-guide/pycuerun-development.md` around lines 100 - 141, The fenced code block containing the component diagram/Class Hierarchy is missing a language tag; edit the opening fence for that block in pycuerun-development.md to include the language identifier "text" (i.e., change ``` to ```text) so syntax highlighting/rendering is correct; locate the block that begins before the bin/pycuerun CLI diagram and update its opening fence (the Class Hierarchy diagram block) accordingly.
220-280:⚠️ Potential issue | 🟡 MinorAdd language tags to fenced code blocks (duplicate issue).
The fenced blocks in the Execution Modes section (lines 226-239, 251-260, 272-280) are missing language identifiers. Add
textafter each opening fence to satisfy MD040.📝 Proposed fix
For each fenced block in this section, change from:
-``` +```text🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/_docs/developer-guide/pycuerun-development.md` around lines 220 - 280, Add explicit language tags to the fenced code blocks in the Execution Modes section: update each opening fence that currently reads ``` (three backticks) to include the language identifier `text` for the command and Flow blocks (patterns like the command block "pycuerun -e 5-render script.outline" and the Flow blocks starting with "options.execute = ..." and "options.inspect = ..."), so all fenced blocks use ```text to satisfy MD040 and ensure consistent linting.
58-96:⚠️ Potential issue | 🟡 MinorAdd language tag to fenced code block (duplicate issue).
This fenced block containing the high-level flow diagram is missing a language identifier and triggers MD040. Add
textafter the opening fence.📝 Proposed fix
-``` +```text pycuerun [options] script.outline [frame_range] │🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/_docs/developer-guide/pycuerun-development.md` around lines 58 - 96, The fenced code block containing the ASCII flow diagram is missing a language tag and triggers MD040; update the opening fence to include the "text" language identifier (i.e., change the ``` fence that begins the diagram to ```text) so the diagram in the pycuerun flow (the block showing PyCuerun, handle_core_args, CuerunOptionParser, handle_my_options and the execute/inspect/launch branches) is properly marked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/_docs/concepts/pycuerun-concepts.md`:
- Around line 63-68: The fenced code block containing the outline pipeline
diagram (starting with the line "pycuerun my_script.outline" and the following
arrow lines) is missing a language tag and triggers MD040; fix it by adding the
language identifier `text` immediately after the opening triple backticks of
that fenced block so the block becomes a ```text fenced code block.
- Around line 87-89: The fenced code block containing "opencue_wrap_frame -->
pycuerun -e {frame}-{layer} script.outline" is missing a language identifier and
triggers MD040; update the opening fence to include the language tag "text"
(i.e., change the opening triple backticks to "```text") so the block becomes a
labeled text code block and satisfies the linter.
- Around line 74-79: The fenced code block showing the pycuerun example is
missing a language identifier and triggers MD040; update the opening fence for
that block (the triple-backtick that precedes the lines "pycuerun -e 5-render
my_script.outline" and the arrow lines) to include the language tag "text" so it
becomes ```text, leaving the block contents unchanged.
In `@docs/_docs/developer-guide/pycuerun-development.md`:
- Around line 396-398: The fenced code block showing the command structure is
missing a language tag; update the opening fence from ``` to ```text so it reads
```text followed by the command line "[strace ...] <wrapper> <user_dir>
<pycuerun> <script> -e `#IFRAME`#-<layer> --version <ver> --repos <repos> --debug
[--dev] [--dev-user <user>]" to ensure the block is annotated as plain text;
modify the block that currently begins with ``` and the matching closing ``` to
use the proposed ```text opening as shown in the diff.
- Around line 170-210: The code block showing the project tree (the fenced block
that begins with the line "pyoutline/") is missing a language identifier; update
the opening fence so it reads ```text (i.e., add the 'text' language tag) for
the fenced block containing the "pyoutline/" tree to ensure proper syntax
highlighting.
- Around line 100-141: The fenced code block containing the component
diagram/Class Hierarchy is missing a language tag; edit the opening fence for
that block in pycuerun-development.md to include the language identifier "text"
(i.e., change ``` to ```text) so syntax highlighting/rendering is correct;
locate the block that begins before the bin/pycuerun CLI diagram and update its
opening fence (the Class Hierarchy diagram block) accordingly.
- Around line 220-280: Add explicit language tags to the fenced code blocks in
the Execution Modes section: update each opening fence that currently reads ```
(three backticks) to include the language identifier `text` for the command and
Flow blocks (patterns like the command block "pycuerun -e 5-render
script.outline" and the Flow blocks starting with "options.execute = ..." and
"options.inspect = ..."), so all fenced blocks use ```text to satisfy MD040 and
ensure consistent linting.
- Around line 58-96: The fenced code block containing the ASCII flow diagram is
missing a language tag and triggers MD040; update the opening fence to include
the "text" language identifier (i.e., change the ``` fence that begins the
diagram to ```text) so the diagram in the pycuerun flow (the block showing
PyCuerun, handle_core_args, CuerunOptionParser, handle_my_options and the
execute/inspect/launch branches) is properly marked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fddc81bd-60d9-474f-9084-5aa33dc11ccc
📒 Files selected for processing (5)
ci/run_integration_test.shdocker-compose.ymldocs/_docs/concepts/pycuerun-concepts.mddocs/_docs/concepts/pyoutline-concepts.mddocs/_docs/developer-guide/pycuerun-development.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docker-compose.yml
…tation - Remove incorrect claim about firing AFTER_INIT events - Document conditional registration via the register arg - Replace nonexistent PluginManager.init_plugin_all() with the actual per-plugin init loop
# Conflicts: # docs/_docs/concepts/pycuerun-concepts.md # docs/_docs/concepts/pyoutline-concepts.md # docs/_docs/developer-guide/pycuerun-development.md # docs/_docs/developer-guide/pyoutline-development.md
…d repos whitespace - Use ~/.opencue/sessions for CI session_dir so mktemp succeeds (owned by runner user) and the path is already bind-mounted into the rqd container - Fix LayerType metaclass docs to match actual behavior (conditional register, no AFTER_INIT events, per-plugin init loop) - Document backend-specific build_command signatures (cue vs local) - Add missing OL_CONFIG to config search order in getting-started and user guide - Add force parameter to put_data in API reference - Fix LayerPreProcess constructor signature (creator, not name) - Update Python version requirement to 3.8+ per pyproject.toml - Fix heading hyphenation in tutorial - Strip whitespace from repos in cue backend build_command
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
ci/run_integration_test.sh (2)
215-220: Session directory and config file not cleaned up.
cleanup()removesRQD_ROOTandVENVbut leavesSESSION_DIR(under${HOME}/.opencue/sessions/) andOUTLINE_CONFIG_FILE(in/tmp/). Over repeated test runs, these temporary resources will accumulate.Proposed fix to clean up session resources
cleanup() { docker compose rm --stop --force >>"${DOCKER_COMPOSE_LOG}" 2>&1 docker volume rm "${DB_VOLUME_NAME}" 2>/dev/null || true rm -rf "${RQD_ROOT}" || true rm -rf "${VENV}" || true + rm -rf "${HOME}/.opencue/sessions" || true + rm -f "${OUTLINE_CONFIG_FILE}" 2>/dev/null || true }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci/run_integration_test.sh` around lines 215 - 220, The cleanup() function currently removes RQD_ROOT and VENV but omits SESSION_DIR and OUTLINE_CONFIG_FILE; update cleanup() to also remove the session directory (SESSION_DIR, e.g., ${HOME}/.opencue/sessions/ or the variable used) and the temporary outline config file (OUTLINE_CONFIG_FILE, e.g., /tmp/...) by adding safe removal commands (rm -rf "${SESSION_DIR}" || true and rm -f "${OUTLINE_CONFIG_FILE}" || true) alongside the existing RQD_ROOT and VENV removals so session state and config files do not accumulate across test runs.
190-208: Emptyhomevalue causes unexpected path interpolation.Setting
home =(empty) means%(home)s/wrappersresolves to/wrappersand%(home)s/binresolves to/bin, which are likely not the intended paths. If these paths are accessed during the test, it could cause failures.Consider either setting a valid
homepath or removing the interpolation references if they're unused in this test scenario.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci/run_integration_test.sh` around lines 190 - 208, The config sets an empty "home" which makes wrapper_dir = %(home)s/wrappers and bin_dir = %(home)s/bin expand to root paths; update the generated config (written to OUTLINE_CONFIG_FILE in the script) to set a valid home (for example home = ${SESSION_DIR}) or remove/replace the %(home)s interpolation for wrapper_dir/bin_dir so they point to a test-local path (use SESSION_DIR or explicit test dirs) to avoid resolving to /wrappers and /bin during tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ci/run_integration_test.sh`:
- Around line 215-220: The cleanup() function currently removes RQD_ROOT and
VENV but omits SESSION_DIR and OUTLINE_CONFIG_FILE; update cleanup() to also
remove the session directory (SESSION_DIR, e.g., ${HOME}/.opencue/sessions/ or
the variable used) and the temporary outline config file (OUTLINE_CONFIG_FILE,
e.g., /tmp/...) by adding safe removal commands (rm -rf "${SESSION_DIR}" || true
and rm -f "${OUTLINE_CONFIG_FILE}" || true) alongside the existing RQD_ROOT and
VENV removals so session state and config files do not accumulate across test
runs.
- Around line 190-208: The config sets an empty "home" which makes wrapper_dir =
%(home)s/wrappers and bin_dir = %(home)s/bin expand to root paths; update the
generated config (written to OUTLINE_CONFIG_FILE in the script) to set a valid
home (for example home = ${SESSION_DIR}) or remove/replace the %(home)s
interpolation for wrapper_dir/bin_dir so they point to a test-local path (use
SESSION_DIR or explicit test dirs) to avoid resolving to /wrappers and /bin
during tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 57188cd2-26eb-4bd6-9b2e-e440eef57c01
📒 Files selected for processing (8)
ci/run_integration_test.shdocs/_docs/developer-guide/pyoutline-development.mddocs/_docs/getting-started/getting-started-pyoutline.mddocs/_docs/quick-starts/quick-start-pycuerun.mddocs/_docs/reference/pyoutline-api-reference.mddocs/_docs/tutorials/pyoutline-tutorial.mddocs/_docs/user-guides/pyoutline-user-guide.mdpyoutline/outline/backend/cue.py
✅ Files skipped from review due to trivial changes (4)
- docs/_docs/user-guides/pyoutline-user-guide.md
- docs/_docs/tutorials/pyoutline-tutorial.md
- docs/_docs/quick-starts/quick-start-pycuerun.md
- docs/_docs/getting-started/getting-started-pyoutline.md
🚧 Files skipped from review as they are similar to previous changes (2)
- pyoutline/outline/backend/cue.py
- docs/_docs/developer-guide/pyoutline-development.md
- Create ~/.opencue/sessions and /tmp/opencue before docker compose up so the CI runner user owns them instead of Docker creating them as root - Fixes mktemp permission denied error when creating session directory - Session dir now lives under ~/.opencue/sessions which is already bind-mounted into the rqd container
ramonfigueiredo
merged commit 2ba7434
into
AcademySoftwareFoundation:master
25 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link the Issue(s) this Pull Request is related to.
pycuerunnot installable via pip, CLI bugs, and sandbox cannot execute frames #2214Summarize your change.
pycuerunas a proper pip-installable entry point viapyproject.toml--priorityoption in CuerunOptionParser--reposargument consuming the next CLI flaguse_pycuerun=Falsecrash (TypeError) for layers without a command argopencue_wrap_frame_sandboxwrapper for environments without tcshdocker-compose.ymlto build rqd from source with volume mounts for frame executionSummary by CodeRabbit
New Features
--priorityjob option and apycuerunconsole commandDocumentation
Chores
Tests
--reposargument in serialized commands