Implement service start and stop with background process and configured executable - #177
Open
mjansson wants to merge 1 commit into
Open
Implement service start and stop with background process and configured executable#177mjansson wants to merge 1 commit into
mjansson wants to merge 1 commit into
Conversation
# Summary The service process could be run in the foreground, but there was no way to start it as a background daemon, stop it, or have clients opt into using it. This adds `lore service start`/`stop`/`set-use-automatically`/`set-executable`, teaches the client to relay through a running service, and sizes the client runtime for relaying rather than local work when it does. A service is only ever started from an executable someone named, either on the call or in the global config. Nothing is inferred from the running executable: with Lore embedded that is the host application, and even for the CLI it would make the answer depend on what the binary happens to be called. Routing therefore takes both settings — a service to route to has to be startable — and `use_service_automatically` without an executable leaves commands running locally rather than failing every one of them. A call routed to a service that could not be reached or started reports a distinct error code, rather than one the caller cannot tell from the command's own failures. ## Changes - `lore/src/remote/process.rs`: spawn and stop the detached service process, with the platform-specific pieces for Unix and Windows. The executable to launch — a path or a `PATH` name — is the one the call names, or failing that the configured `service_executable`; with neither, there is nothing to start and the call says so. The global config is read only when a service actually has to be started, so the common path does not touch it. Spawning keeps the child so a process that exits instead of listening is reported at once rather than after the start timeout; one that exits because another service took the socket first is success, since a service is listening. - `lore/src/service.rs`, `lore/src/interface.rs`, `lore/src/lib.rs`: the `start`, `stop`, `set_use_automatically` and `set_executable` operations, routed as no-repository calls so they run locally rather than relaying. Starting a running service and stopping a stopped one are both success, so a caller need not check first. Writing the executable drops the cached routing decision, which now depends on it as well as on the routing setting. - `lore-revision/src/global.rs`: `service_executable`, the executable to launch when a call names none. Blank reads as unset, so a stored empty string cannot become a command with no name. `lore service set-executable` writes it and an empty value clears it. - `lore-base/src/error.rs`, `lore-revision/src/interface.rs`, `lore-revision/src/event.rs`: `ServiceUnavailable` (FFI code 50) for a call routed to a service that could not be reached or started. The general and per-event enums share its value, so the number is the same whether it arrives as a return value or on an event. - `lore/src/remote/call.rs`, `lore/src/call_delegation.rs`: decide once per process whether calls relay, rather than opening a probe connection on each call, and size the runtime lean on the first relayed FFI call. Relaying takes `use_service_automatically` and an executable to start a service from: the routing setting alone would leave every call reporting that none could be started, so on its own it leaves them running locally instead. The serialized command is shared rather than copied, since the send is retried only when a service had to be started first and the payload is as large as the command. Routed failures return the error's own code, where a fixed `1` had collided with `InvalidArguments`. A closed connection reports that the service stopped instead of a generic transport error. - `lore/src/remote.rs`: `LORE_SERVICE_SOCKET` names a socket of a process group's own. There is otherwise one service per user, named the same for everyone, so a service started against one global config would serve calls made against another — which is what kept the test suite from starting real services. The value is one file-name component; anything that would leave the socket directory is refused in favour of the default. - `scripts/test/conftest.py`, `scripts/test/lore.py`: give the Python suite a global config and credential store of its own. Routing now follows `use_service_automatically`, so a command that reads the developer's global config rather than the test's behaves differently on their machine than in CI: with the setting enabled it runs in a service, which a test that kills the client does not stop. Commands run through the wrapper were already isolated; a session-wide default now covers what is not — a raw `Popen` of the binary, and the C API called in the test process, which has no command to set it on — and `sandboxed_env` gives the raw spawns the per-test directory. The per-test credential store is assigned rather than defaulted: the session-wide value is always present by then, so a default would leave every test sharing one token store instead of holding its own. - `scripts/test/test_compact.py`: the interrupted `repository gc` runs in the test's own global config too, rather than sharing the run-wide one with every other worker. - `scripts/test/test_manyfiles.py`: the interrupted-commit test no longer assumes the kill lands before the commit records its revision. A sleep cannot pin that down, so it stages again when the stage is already gone, and the recovery commit it exists to check runs either way. - `lore-integration-tests/tests/integration.rs`: give the test binary a global config and credential store of its own. Its tests call the same entry points a user does, and those read the global config — `repository::create` consults `use_shared_store_automatically`, and every routed call consults `use_service_automatically` — so without this they read, and could write, whatever the developer running them has configured. A constructor sets it, since the setting is process-wide and this runs before the harness starts the threads that would make writing to the environment a race. It names a service socket of its own for the same reason. - `lore-integration-tests/tests/service_test.rs`: the service C API exercised the way an embedder calls it, which the CLI-driven tests cannot reach — including that a configured executable is what a start naming none launches. Each case asserts on the reported message as well as the status, since every failure here reports the same status and that alone would pass whether or not the named executable was used. The cases that depend on the configured executable hold a lock while they write it: it is one file shared by every test in the process. - `lore-base/src/runtime.rs`, `lore-client/src/cli/client_main.rs`: size the client's threading for relaying to the service instead of for local work. - `LORE_USE_SERVICE` accepts the usual off values (`0`, `false`, `no`, `off`). - `lore-revision/src/global.rs`: isolate the service's global config so tests can cover config routing. - Docs: a tutorial for running commands through the service, which now starts by naming the executable to run as one; the CLI command and config reference entries, including that routing takes both settings; and notes on the single-service-per-process constraint, the shutdown wake-up dependency, that shutdown does not drain in-flight requests, that relay sizing is one-shot, and the `use-service` cache staleness limits. # Test Plan - `cargo test -p lore --lib`, `lore/tests/dispatch_runtime.rs`: executable resolution for the launched service, and client runtime sizing for relaying vs local work. - `scripts/test/test_service.py`: start/stop, starting from a named executable, writing and clearing the configured one, the service-unavailable code on a routed call, graceful shutdown, concurrent stop, and config routing; pinned to a single test worker. Both halves of the routing rule are covered: the setting without an executable runs locally, and with one the command starts a service and runs there. That it ran there rather than locally is shown by taking the service away — with the configured executable pointed at something that cannot start, the command keeps working while a service is listening and reports service-unavailable once it is stopped. The suite's own service socket is what makes starting a real service safe here, without meeting whatever is running on the machine. - `uv run pytest scripts/test -m smoke -n 4` against release binaries: 883 passed. The three that failed before the sandbox — an interrupted commit, two notification subscriptions, and a C API call reporting the service unavailable — were all reading the developer's global config. - `cargo test -p lore-integration-tests --features integration_tests` against the compose stack: 324 passed. A test asserts the global config resolves inside the sandbox, and a run against a config with the service and shared stores enabled fails without the sandbox and passes with it. - Every command in the new tutorial run by hand against an isolated `LORE_GLOBAL_PATH`, including the two cases it now calls out: forcing the service on without an executable reports service-unavailable, and the routing setting without one runs the command locally. - `cargo clippy --all-targets -- -D warnings --no-deps` and `cargo +nightly fmt --all --check`: clean. Signed-off-by: Mattias Jansson <mjansson@gmail.com>
github-actions
Bot
added
documentation
epic-lore-bot
Bot
added
imported
|
Imported as Lore CR-462. |
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
1 participant
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.
Summary
The service process could be run in the foreground, but there was no way to start it as a background process, stop it, or have clients opt into using it. This implements
lore service start/stop/set-use-automatically, teaches the client to relay through a running service, and sizes the client runtime for relaying rather than local work when it does.Starting names the executable to launch, so that a caller with Lore embedded can start a service. A call routed to a service that could not be reached or started reports a distinct error code, rather than one the caller cannot tell from the command's own failures.
Automatic launch of a background process is supported through two new global config items: Enabling the automatic use, and which executable to use to launch the background process.
Adds tutorial docs on how to configure and use the service process.
Updated test harness to ensure tests run in a sandboxed environment where the machine global config does not interfere.
Test Plan
Ran the tutorial commands manually on a clean setup using an isolated
LORE_GLOBAL_PATHand verified all steps worked.Ran full test suite and verified there were no regressions.
Ran vale on the documentation and verified it passed all rules.