Skip to content

fix(cli): handle spaces in the Python path when starting the server - #965

Open
enekoreto wants to merge 1 commit into
apache:mainfrom
enekoreto:fix/cli-paths-with-spaces
Open

enekoreto wants to merge 1 commit into
apache:mainfrom
enekoreto:fix/cli-paths-with-spaces

Conversation

@enekoreto

@enekoreto enekoreto commented Oct 15, 2026 •

Copy link
Copy Markdown

Fixes burr failing to start when the Python path contains a space.

Closes #964

Changes

  • _run_server builds the uvicorn command as a list instead of a string. It still uses sys.executable like fix: address RC2 vote feedback #745 did.
  • _command now accepts a list. The run calls were inside the isinstance(command, str) branch, so a list did nothing before. Strings are still split like before.

How I tested this

  • Added tests/test_cli.py. Both tests fail on main.
  • Ran burr --no-open from my venv under Apache burr/. It fails on main and starts with this change, also with --dev-mode.
  • Full test suite passes locally.

Notes

CI doesn't install the cli extra (loguru), so the new tests are skipped there. I ran them locally on 3.10 and 3.13.

#911 also edits the _command signature line, so whichever of the two goes in second will need a small rebase.

I used Claude Code while working on this; I've reviewed and tested the change myself.

Checklist

  • PR has an informative and human-readable title (this will be pulled into the release notes)
  • Changes are limited to a single goal (no scope creep)
  • Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future TODOs are captured in comments
  • Project documentation has been updated if adding/changing functionality.

@elijahbenizzy elijahbenizzy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fix is right, thanks -- building argv as a list is the correct repair, and the list branch in _command was dead on main. One blocker before I approve: the signature still has addl_env: dict | None, which fails at import on 3.9 (CI still targets it, and the CLI tests skip there so CI won't catch it). #911 changes that same line to Optional[dict] and is merging first; please rebase onto it once it lands and keep its version. Elijah

@enekoreto
enekoreto force-pushed the fix/cli-paths-with-spaces branch from 6e7af8d to 5882023 Compare October 17, 2026 15:33
@enekoreto

Copy link
Copy Markdown
Author

Thanks Elijah! Rebased onto main now that #911 is in, and kept its Optional[dict] for addl_env.

This branch has not been deployed

No deployments
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.

burr fails to start when the Python path has a space in it

2 participants