mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-13 18:15:02 +00:00
fix: run the module when the docsgpt command is not on PATH
Refusing to write the service units when `docsgpt` is not on PATH was wrong. A package installed in a virtualenv is runnable whether or not its console script is on PATH, and CI runs pytest as `python -m pytest`, where argv[0] is a module file: the refusal failed thirteen native tests there. The launcher now prefers the command on PATH, resolved to an absolute path since PATH can hold relative entries, then an argv[0] that can be executed, and otherwise this interpreter with `-m docsgpt`, which works wherever the package is importable. `python -m docsgpt` became an entrypoint of its own and has a test that runs it.
This commit is contained in:
1 parent
f9f52e99f0
commit
a96734da07
5 files changed
+53
-20
No files matched your search
@@ -0,0 +1,8 @@
|
||||
"""``python -m docsgpt`` runs what the ``docsgpt`` command runs."""
|
||||
|
||||
import sys
|
||||
|
||||
from docsgpt.cli import main
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
@@ -218,13 +218,24 @@ def _redis_urls(base: str) -> dict[str, str]:
|
||||
}
|
||||
|
||||
|
||||
def _own_executable() -> str:
|
||||
"""The docsgpt command to put in the service files, or a DeployError naming what to install."""
|
||||
def _native_launcher() -> list[str]:
|
||||
"""How the service files start DocsGPT: the command on PATH, or this interpreter and the module.
|
||||
|
||||
A unit has to name something that can be executed, and ``sys.argv[0]`` often cannot be: under
|
||||
``python -m docsgpt``, or pytest, it is a module file. Falling back to the running interpreter
|
||||
works wherever the package is importable, which it must be to have got here.
|
||||
"""
|
||||
found = shutil.which("docsgpt")
|
||||
if found:
|
||||
# Absolute: PATH can hold relative entries, and a unit file needs a program it can exec.
|
||||
return [str(Path(found).resolve())]
|
||||
candidate = Path(sys.argv[0]).resolve()
|
||||
if candidate.is_file() and os.access(candidate, os.X_OK):
|
||||
return str(candidate)
|
||||
return [str(candidate)]
|
||||
if sys.executable:
|
||||
return [sys.executable, "-m", "docsgpt"]
|
||||
raise DeployError(
|
||||
"the docsgpt command is not on PATH, so the services would have nothing to run. "
|
||||
"could not work out how to start docsgpt for the services. "
|
||||
"Install it with `uv tool install docsgpt` (or `pip install docsgpt`) and run this again."
|
||||
)
|
||||
|
||||
@@ -274,14 +285,14 @@ def _native_up(args, context: Context, directory: Path) -> int:
|
||||
(directory / "logs").mkdir(exist_ok=True)
|
||||
envfile.update(env_path, updates)
|
||||
|
||||
executable = shutil.which("docsgpt") or _own_executable()
|
||||
launcher = _native_launcher()
|
||||
print("Applying database migrations ...")
|
||||
# The child reads the stack's settings, not a .env in whatever directory this was run from.
|
||||
stack_env = {"DOCSGPT_HOME": str(directory), "DOCSGPT_ENV_FILE": str(env_path)}
|
||||
if context.run([executable, "migrate"], stack_env) != 0:
|
||||
if context.run([*launcher, "migrate"], stack_env) != 0:
|
||||
raise DeployError("`docsgpt migrate` failed; check the database URL and that the server is reachable.")
|
||||
|
||||
for unit in native.units_for(directory, executable, port, directory):
|
||||
for unit in native.units_for(directory, launcher, port, directory):
|
||||
services.install(unit)
|
||||
for name in native.SERVICES:
|
||||
print(f"Starting {name} ...")
|
||||
|
||||
@@ -204,20 +204,23 @@ def services_for_platform(platform: str = sys.platform):
|
||||
)
|
||||
|
||||
|
||||
def units_for(stack_directory: Path, executable: str, port: int, home: Path) -> list[Unit]:
|
||||
"""The API and worker services for a native install in ``stack_directory``."""
|
||||
def units_for(stack_directory: Path, launcher: list[str], port: int, home: Path) -> list[Unit]:
|
||||
"""The API and worker services for a native install in ``stack_directory``.
|
||||
|
||||
``launcher`` is how DocsGPT is started: the ``docsgpt`` command, or an interpreter and ``-m``.
|
||||
"""
|
||||
environment = {"DOCSGPT_HOME": str(home)}
|
||||
return [
|
||||
Unit(
|
||||
name=API_SERVICE,
|
||||
arguments=[executable, "api", "--host", "127.0.0.1", "--port", str(port)],
|
||||
arguments=[*launcher, "api", "--host", "127.0.0.1", "--port", str(port)],
|
||||
environment=dict(environment),
|
||||
working_directory=str(stack_directory),
|
||||
log_file=str(stack_directory / "logs" / "api.log"),
|
||||
),
|
||||
Unit(
|
||||
name=WORKER_SERVICE,
|
||||
arguments=[executable, "worker"],
|
||||
arguments=[*launcher, "worker"],
|
||||
environment=dict(environment),
|
||||
working_directory=str(stack_directory),
|
||||
log_file=str(stack_directory / "logs" / "worker.log"),
|
||||
|
||||
@@ -90,8 +90,8 @@ class TestNativeUp:
|
||||
"--postgres-uri", "postgresql://localhost/docsgpt"], _native_context(services)) == 0
|
||||
api = services.units["docsgpt-api"]
|
||||
worker = services.units["docsgpt-worker"]
|
||||
assert api.arguments[1:] == ["api", "--host", "127.0.0.1", "--port", "7091"]
|
||||
assert worker.arguments[1:] == ["worker"]
|
||||
assert api.arguments[-5:] == ["api", "--host", "127.0.0.1", "--port", "7091"]
|
||||
assert worker.arguments[-1] == "worker"
|
||||
for unit in (api, worker):
|
||||
assert unit.environment["DOCSGPT_HOME"] == str(tmp_path)
|
||||
assert unit.working_directory == str(tmp_path)
|
||||
@@ -105,7 +105,7 @@ class TestNativeUp:
|
||||
assert _run(["up", "--native", "--dir", str(tmp_path), "--yes",
|
||||
"--postgres-uri", "postgresql://localhost/docsgpt"], context) == 0
|
||||
arguments, environment = calls[0]
|
||||
assert arguments[1] == "migrate"
|
||||
assert arguments[-1] == "migrate", "the launcher can be an interpreter and -m docsgpt"
|
||||
assert environment["DOCSGPT_HOME"] == str(tmp_path)
|
||||
assert environment["DOCSGPT_ENV_FILE"] == str(tmp_path / ".env")
|
||||
|
||||
@@ -139,13 +139,14 @@ class TestNativeUp:
|
||||
with pytest.raises(DeployError, match="holds a Docker install"):
|
||||
_run(argv, _native_context())
|
||||
|
||||
def test_services_are_not_written_without_a_docsgpt_command(self, tmp_path, monkeypatch):
|
||||
"""A unit pointing at something unexecutable would fail to exec with only a health check to show it."""
|
||||
def test_without_the_command_on_path_the_services_run_the_module(self, tmp_path, monkeypatch):
|
||||
"""A unit must name something executable; argv[0] is a module file under `python -m`."""
|
||||
monkeypatch.setattr("docsgpt.deploy.commands.shutil.which", lambda name: None)
|
||||
monkeypatch.setattr(sys, "argv", [str(tmp_path / "not-a-program")])
|
||||
services = FakeServices()
|
||||
argv = ["up", "--native", "--dir", str(tmp_path), "--yes", "--postgres-uri", "postgresql://localhost/d"]
|
||||
with pytest.raises(DeployError, match="not on PATH"):
|
||||
_run(argv, _native_context())
|
||||
assert _run(argv, _native_context(services)) == 0
|
||||
assert services.units["docsgpt-api"].arguments[:3] == [sys.executable, "-m", "docsgpt"]
|
||||
|
||||
def test_a_database_url_is_required(self, tmp_path):
|
||||
with pytest.raises(DeployError, match="--postgres-uri"):
|
||||
@@ -292,7 +293,7 @@ class TestSystemd:
|
||||
def test_install_writes_the_unit_and_reloads(self, tmp_path):
|
||||
systemctl = FakeSystemctl()
|
||||
services = self._services(systemctl, tmp_path)
|
||||
services.install(native.units_for(tmp_path, "/venv/bin/docsgpt", 7091, tmp_path)[0])
|
||||
services.install(native.units_for(tmp_path, ["/venv/bin/docsgpt"], 7091, tmp_path)[0])
|
||||
unit = tmp_path / ".config" / "systemd" / "user" / "docsgpt-api.service"
|
||||
assert "ExecStart=/venv/bin/docsgpt api" in unit.read_text()
|
||||
assert systemctl.verbs == ["daemon-reload"]
|
||||
@@ -308,7 +309,7 @@ class TestSystemd:
|
||||
def test_stop_and_remove(self, tmp_path):
|
||||
systemctl = FakeSystemctl()
|
||||
services = self._services(systemctl, tmp_path)
|
||||
services.install(native.units_for(tmp_path, "/venv/bin/docsgpt", 7091, tmp_path)[0])
|
||||
services.install(native.units_for(tmp_path, ["/venv/bin/docsgpt"], 7091, tmp_path)[0])
|
||||
unit = tmp_path / ".config" / "systemd" / "user" / "docsgpt-api.service"
|
||||
services.stop("docsgpt-api")
|
||||
assert unit.is_file(), "stopping keeps the unit"
|
||||
|
||||
@@ -34,6 +34,16 @@ class TestTopLevel:
|
||||
subprocess.run([sys.executable, "-c", code], cwd=Path(__file__).resolve().parents[1], check=True)
|
||||
|
||||
|
||||
class TestModuleEntrypoint:
|
||||
def test_python_m_docsgpt_runs_the_cli(self):
|
||||
"""`python -m docsgpt` is what a native service falls back to when the script is not on PATH."""
|
||||
result = subprocess.run(
|
||||
[sys.executable, "-m", "docsgpt", "--version"],
|
||||
cwd=Path(__file__).resolve().parents[1], capture_output=True, text=True, check=True,
|
||||
)
|
||||
assert result.stdout.strip() == f"docsgpt {__version__}"
|
||||
|
||||
|
||||
class TestHome:
|
||||
@staticmethod
|
||||
def _installed(monkeypatch, tmp_path):
|
||||
|
||||
Reference in new issue
Block a user