Prepare applications before server readiness (#642) - #678
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Warning Your free Security trial is over. An organization admin can activate Security or dismiss this notice. Comment |
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Reviewer's GuideThe server now builds and prepares its application once before readiness, propagates a shared immutable prepared root through workers and connection tasks, reports startup failures as typed errors, and updates APIs, examples, documentation, tests, and benchmarks for the new lifecycle contract. Sequence diagram for prepared application server startupsequenceDiagram
participant Caller
participant Server as WireframeServer
participant Factory as AppFactory
participant App as WireframeApp
participant Workers as AcceptLoops
participant Connection as ConnectionTask
Caller->>Server: run_with_shutdown()
Server->>Factory: build()
Factory-->>Server: WireframeApp
Server->>App: prepare()
App-->>Server: PreparedApp
Server->>Workers: install accept loops with Arc<PreparedApp>
Server-->>Caller: readiness signal
Workers->>Connection: spawn_connection_task(Arc<PreparedApp>)
Connection->>App: handle_connection_result()
State diagram for server readiness and startup failuresstateDiagram-v2
[*] --> Unbound
Unbound --> Bound: bind()
Bound --> Starting: run_with_shutdown()
Starting --> FactoryBuildFailed: build() error
Starting --> PreparationFailed: prepare() error
Starting --> Ready: workers installed
Ready --> Running: readiness signalled
FactoryBuildFailed --> [*]
PreparationFailed --> [*]
Running --> Stopping: shutdown
Stopping --> [*]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Evaluate and prepare the startup app before accept loops signal readiness. Share one immutable PreparedApp root with every worker and connection task, while retaining shutdown tracking and panic isolation. Document once-per-run factory behaviour, add from_app, and move bundled examples onto the server runtime path. Add startup-count benchmarks and tests.
Hold middleware preparation behind a barrier and prove that the server does not signal readiness until preparation completes. Keep the runtime test module within the repository's 400-line limit.
f03af46 to
e606ea1
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e606ea1014
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| server run, prepares the result once, and shares that immutable application | ||
| root across connections. Move per-connection state to `on_connection_setup`, | ||
| or use `WireframeServer::from_app(app)` when the application is already | ||
| built. See `docs/v0-2-0-to-v0-3-0-migration-guide.md` for migration examples. |
There was a problem hiding this comment.
This unreleased breaking change belongs to the v0.3.0-to-v0.4.0 transition, but the link sends users to the already-published v0.2.0-to-v0.3.0 guide. More importantly, docs/v0-3-0-to-v0-4-0-migration-guide.md still says at lines 99–104 that factories retain per-connection evaluation and that this work is deferred to issue #642. A v0.3 user following the designated current guide will therefore implement the opposite lifecycle semantics; update that guide and link to it here.
Useful? React with 👍 / 👎.
| `WireframeServer::from_app` for tracing setup, server preparation, listener | ||
| binding, connection spawning, shutdown-aware accept loops, and current-thread | ||
| Tokio runtime startup. Keep example-specific address |
There was a problem hiding this comment.
from_app
WireframeServer::from_app only stores the application in a server builder; it does not initialize tracing, create a Tokio runtime, or parse configuration. The updated examples still perform those steps explicitly with try_init() and tokio::runtime::Builder, so this convention would cause a contributor following the developers' guide to omit required bootstrap code. Describe from_app as replacing application preparation and server-loop wiring while retaining explicit tracing and runtime setup.
Useful? React with 👍 / 👎.
1 participant
Closes #642
Summary
The server now evaluates its startup factory once, prepares one immutable root before installing accept loops, and shares that root with every connection task. Startup failures now surface as typed server errors before readiness can be signalled.
Implementation
Arc<PreparedApp>through the runtime, accept loop, and connection spawner.from_appfor a pre-built application.C; adds factory/transform startup benchmarks and regression coverage.Validation
make check-fmtmake lintmake typecheckmake testmake markdownlintmake nixieReferences
Summary by Sourcery
Prepare and share the application before installing server workers so startup failures occur before readiness and connections reuse one immutable root.
New Features:
WireframeServer::from_appfor serving an already-built application.Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores: