gorums: rename the public API and fold System into Server - #331
Open
meling wants to merge 16 commits into
Open
Conversation
meling
changed the title
Contributor
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Go |  |
Aug 12, 2026 12:38p.m. | Review ↗ |
| Shell | |
Aug 12, 2026 12:38p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR performs a broad Gorums API rename/refactor across the core library, test helpers, examples, docs, and code generation templates—primarily shifting terminology from “Configuration” → “Config”, clarifying interceptor types (client vs server), renaming low-level call helpers (e.g., RPCCall → RemoteCall), and simplifying server lifecycle management by removing the System abstraction in favor of Server methods and NewLocalServers.
Changes:
- Renamed and reshaped core APIs:
Configuration→Config,NodeListOption→NodeSource,ServerCtx→ServerContext,Interceptor→ServerInterceptor,QuorumInterceptor→ClientInterceptor,RPCCall→RemoteCall, andWithDialOptions→WithGRPCDialOptions. - Replaced
System/NewSystem/NewLocalSystemswithServer.WithAddr+Server.ListenAndServe+NewLocalServers, and updated examples/tests accordingly. - Updated protoc-gen-gorums templates, reserved identifiers, generated test protos, and documentation to match the new surface.
Reviewed changes
Copilot reviewed 78 out of 89 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| system.go (deleted) | Removes System wrapper API in favor of Server lifecycle APIs. |
| server.go | Renames server interceptor APIs, adds listener tracking (Addr, ListenAndServe), updates peer/config types. |
| server_test.go | Updates tests to new names (ServerContext, RemoteCall, WithServerInterceptors). |
| server_handler.go | Renames request/handler context and message fields (Msg → Proto), updates interceptor types. |
| server_handler_test.go | Updates message field assertions and context type names. |
| responses.go | Updates constructor to use CallContext. |
| responses_test.go | Updates helper to build CallContext and Config. |
| remote_call.go | Renames RPCCall → RemoteCall. |
| remote_call_test.go | Updates remote call tests to RemoteCall. |
| quorumcall.go | Switches to newQuorumCallContext constructor naming. |
| outbound_manager.go (new) | Introduces/relocates outbound connection pool management under new API naming. |
| options_test.go | Updates expectations around nil option handling and metadata behavior naming. |
| node.go | Renames comparators to By* and updates docs to Config.Sort. |
| node_test.go | Updates comparator usage to ByID, ByLastError, ByLatency and Config. |
| node_source.go | Renames NodeListOption → NodeSource, updates config building and sorting comparator. |
| multicast.go | Switches to newMulticastCallContext naming. |
| local_servers.go (new) | Replaces NewLocalSystems with NewLocalServers and moves listener allocation there. |
| inbound_manager.go | Renames configuration types to Config, updates peer/client config and callback signatures. |
| inbound_manager_test.go | Updates tests for NodeSource, Config, dial option renames, and handler context type. |
| dial_options.go | Renames dial option helper to WithGRPCDialOptions, removes WithServerOptions (System removed). |
| config.go | Renames Configuration → Config, SortBy → Sort, and updates method names/docs accordingly. |
| config_test.go | Updates all config tests to Config, NodeSource, and new sort/comparator API. |
| errors.go | Renames QuorumCallError.NodeErrors() → NumErrors(). |
| errors_test.go | Updates tests to new NumErrors() accessor. |
| callopts.go | Renames quorum/client interceptor types (QuorumInterceptor → ClientInterceptor). |
| callopts_test.go | Replaces in-package System-based test setup with NewLocalServers + ListenAndServe, updates interceptor ctx types. |
| call_quorum_test.go | Updates error accessor to NumErrors(). |
| call_context.go | Renames ClientCtx → CallContext, updates interceptor types and multicast/quorum context constructors. |
| call_client_interceptor_test.go | Updates custom interceptor types to ClientInterceptor/CallContext. |
| call_correctable_test.go (new) | Adds correctable quorum call tests/benchmarks using new APIs. |
| call_async_test.go (new) | Adds async terminal method tests/benchmarks using new APIs. |
| gorums.proto | Renames method option rpc → remotecall. |
| gorums.pb.go | Regenerates protobuf extensions to match remotecall option rename. |
| cmd/protoc-gen-gorums/gengorums/template_static.go | Updates reserved identifiers and static aliases to Config. |
| cmd/protoc-gen-gorums/gengorums/template_server.go | Updates generated server context type to ServerContext. |
| cmd/protoc-gen-gorums/gengorums/template_rpc.go | Renames RPC call template to use RemoteCall and updated identifiers. |
| cmd/protoc-gen-gorums/gengorums/gorums.go | Updates call type registration from rpc to remotecall and swaps templates. |
| cmd/protoc-gen-gorums/gengorums/gorums_bundle_test.go | Updates reserved identifier expectations (Config replaces Configuration). |
| cmd/protoc-gen-gorums/dev/aliases.go | Updates dev aliases to export Config. |
| cmd/protoc-gen-gorums/dev/generated_code_test.go | Updates handler context types in generated-code tests. |
| cmd/protoc-gen-gorums/dev/zorums.pb.go | Regenerated with updated protoc version metadata. |
| cmd/protoc-gen-gorums/dev/zorums_server_gorums.pb.go | Regenerated server-side API to ServerContext and new aliases. |
| cmd/protoc-gen-gorums/dev/zorums_remotecall_gorums.pb.go | Regenerated remote call wrapper to RemoteCall. |
| cmd/protoc-gen-gorums/dev/zorums_unicast_gorums.pb.go | Regenerated with updated aliases/version metadata. |
| cmd/protoc-gen-gorums/dev/zorums_multicast_gorums.pb.go | Regenerated with updated aliases/version metadata. |
| cmd/protoc-gen-gorums/dev/zorums_quorumcall_gorums.pb.go | Regenerated with updated aliases/version metadata. |
| cmd/protoc-gen-gorums/dev/zorums_types_gorums.pb.go | Regenerated with updated aliases/version metadata. |
| internal/stream/stream.pb.go | Regenerated with updated protoc version metadata. |
| internal/stream/stream_grpc.pb.go | Regenerated with updated protoc version metadata. |
| internal/tests/unresponsive/unresponsive.pb.go | Regenerated with updated protoc version metadata. |
| internal/tests/unresponsive/unresponsive_gorums.pb.go | Updates generated aliases (Config) and RemoteCall/ServerContext. |
| internal/tests/unresponsive/unreponsive_test.go | Updates handler context type to ServerContext. |
| internal/tests/tls/tls.pb.go | Regenerated with updated protoc version metadata. |
| internal/tests/tls/tls_gorums.pb.go | Updates generated aliases (Config) and RemoteCall/ServerContext. |
| internal/tests/tls/tls_test.go | Updates handler context type and dial option helper rename. |
| internal/tests/ordering/order.pb.go | Regenerated with updated protoc version metadata. |
| internal/tests/ordering/order_gorums.pb.go | Updates generated aliases (Config) and RemoteCall/ServerContext. |
| internal/tests/ordering/order_test.go | Updates handler context type to ServerContext. |
| internal/tests/oneway/oneway.pb.go | Regenerated with updated protoc version metadata. |
| internal/tests/oneway/oneway_gorums.pb.go | Updates generated aliases (Config) and ServerContext. |
| internal/tests/oneway/oneway_test.go | Updates handler context type and config alias to Config. |
| internal/tests/metadata/metadata.pb.go | Regenerated with updated protoc version metadata. |
| internal/tests/metadata/metadata_gorums.pb.go | Updates generated aliases (Config) and RemoteCall/ServerContext. |
| internal/tests/metadata/metadata_test.go | Updates handler context type to ServerContext. |
| internal/tests/correctable/correctable.pb.go | Regenerated with updated protoc version metadata. |
| internal/tests/correctable/correctable_gorums.pb.go | Updates generated aliases (Config) and ServerContext. |
| internal/tests/correctable/correctable_test.go | Updates handler context type and stream signature. |
| internal/tests/config/config.proto | Renames RPC method Config → Read in test proto. |
| internal/tests/config/config.pb.go | Regenerated to reflect Read method rename and updated version metadata. |
| internal/tests/config/config_gorums.pb.go | Regenerated quorum call wrapper Read(...) and updates ServerContext/Config alias. |
| internal/tests/config/config_test.go | Updates server impl method name and config alias to Config. |
| internal/testprotos/failing/reservednames/reserved.proto | Updates reserved-name failure proto to use Config instead of Configuration. |
| gorumstest/gorumstest.go | Renames helper Systems → LocalServers, updates return types to Config, dial helper rename. |
| gorumstest/options.go | Updates test option extraction to NodeSource and terminology changes. |
| gorumstest/servers.go | Updates handler context type to ServerContext. |
| examples/storage/server.go | Migrates example from System to Server.WithAddr + ListenAndServe and NewLocalServers. |
| examples/storage/client.go | Updates dial helper rename to WithGRPCDialOptions. |
| examples/storage/main.go | Updates interceptor type and option name (WithServerInterceptors). |
| examples/storage/repl.go | Updates config alias, sorting comparator rename (ByID). |
| examples/storage/proto/storage.pb.go | Regenerated with updated protoc version metadata. |
| examples/storage/proto/storage_gorums.pb.go | Regenerated with Config alias + RemoteCall + ServerContext. |
| examples/interceptors/server_interceptors.go | Updates server interceptor signature/types (ServerInterceptor, ServerContext). |
| doc/user-guide.md | Updates documentation to new naming, interceptor types, and server/config APIs. |
| doc/ordering.md | Updates handler context type naming. |
| doc/migration-v2-to-v3.md | Updates migration guide terminology and dial helper rename. |
| doc/dev-guide.md | Updates generated-alias docs to Config. |
| doc/design-doc-layering.md (deleted) | Removes an older design doc (layering). |
| AGENTS.md | Updates wording from “Configuration” to “Config” in one place. |
Files not reviewed (8)
- cmd/protoc-gen-gorums/dev/zorums.pb.go: Generated file
- cmd/protoc-gen-gorums/dev/zorums_multicast_gorums.pb.go: Generated file
- cmd/protoc-gen-gorums/dev/zorums_quorumcall_gorums.pb.go: Generated file
- cmd/protoc-gen-gorums/dev/zorums_server_gorums.pb.go: Generated file
- cmd/protoc-gen-gorums/dev/zorums_types_gorums.pb.go: Generated file
- cmd/protoc-gen-gorums/dev/zorums_unicast_gorums.pb.go: Generated file
- cmd/protoc-gen-gorums/gengorums/gorums.go: Generated file
- cmd/protoc-gen-gorums/gengorums/template_static.go: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| WriteUnicast(ctx gorums.ServerContext, request *WriteRequest) | ||
| WriteMulticast(ctx gorums.ServerContext, request *WriteRequest) | ||
| ReadQC(ctx gorums.ServerContext, request *ReadRequest) (response *ReadResponse, err error) | ||
| ReadCorrectable(ctx gorums.ServerContext, request *ReadRequest, send func(response *ReadResponse) error) error |
meling
force-pushed
the
gorums/api-renames
branch
from
August 12, 2026 11:42
2b3c71c to
37fb53f
Compare
System wrapped a Server, a listener, and a list of closers. Every method on it either delegated to the Server or managed the listener, so a user had two types to reason about, two Stop methods, and two places to look for the peer configuration. Registering a service went through a callback that received the Server the caller already had. Server now owns its listener. WithAddr records the address, ListenAndServe binds it, Serve accepts an externally supplied one, Addr reports whichever is in effect, and Stop closes it. NewLocalServers replaces NewLocalSystems and returns servers with their listeners preallocated, so a test knows every address before anything serves. Services register directly on the Server, which is what the generated registration functions already take. The closer list goes with System: a caller that needs a resource closed on shutdown can close it itself, and nothing in the repository used the returned error. WithServerOptions goes too. It existed only to smuggle ServerOptions through NewSystem's dial-option list, and NewServer takes them directly. system_test.go becomes server_e2e_test.go. Its two closer-registry tests are removed with the feature they tested, and the listener test that only applied to NewSystem's eager bind now covers NewLocalServers instead.
Configuration is the longest identifier in the public API and appears in almost every signature. Config says the same thing, matches ConfigContext and NewConfig which already used the shorter form, and removes the mismatch where a type named Configuration was constructed by a function named NewConfig. ConfigContext.Configuration becomes ConfigContext.Config for the same reason. Config joins the reserved identifiers injected into generated code, so a proto message named Config now collides where one named Configuration used to. Two test protos are adjusted: the reserved-name failure fixture asserts on the new name, and internal/tests/config renames its Config RPC to Read, which also reads better for a quorum call returning a value.
ServerCtx was the only abbreviated type in the public API. It embeds a context.Context and is the server-side counterpart of ConfigContext and NodeContext, neither of which abbreviates, so the short form was inconsistent rather than concise. Generated handler signatures carry this type, so the name appears in every service implementation.
The type is not an option. It is a value that produces the nodes of a configuration, implemented by WithNodes for a map and WithNodeList for an address slice, and it is a required argument to NewConfig and WithPeers rather than an optional one. Calling it an Option put it in the same category as DialOption and ServerOption, which are functional options that may be omitted. NodeSource says what it is: the source the configuration draws its nodes from.
WithDialOptions took grpc.DialOptions and returned a gorums.DialOption, so a reader had to know which of the two "dial options" was meant at each call site. It becomes WithGRPCDialOptions, which names the options it forwards. Interceptor and WithInterceptors are server-side only, but nothing in the names said so, and a client-side interceptor type exists on the call path. They become ServerInterceptor and WithServerInterceptors.
NodeErrors returns a count, not the errors. A caller reading err.NodeErrors() would reasonably expect a slice of node errors, which is what Unwrap provides. NumErrors states that it is a count.
SortBy read as though it took a key, not a comparison function; Sort matches slices.Sort and slices.SortFunc, which is what it delegates to. The comparators ID, Latency, and LastNodeError shared their names with the Node methods they read, so ID could mean either the comparator or the accessor depending on context, and a test sorting by latency had to write slices.SortFunc(nodes, Latency) next to node.Latency(). They become ByID, ByLatency, and ByLastError, which read correctly at the call site: c.Sort(ByLatency).
RPCCall repeats itself: the C in RPC already stands for call. RemoteCall keeps the distinction from the in-process local node path without the stutter, and sits alongside QuorumCall, Multicast, and Unicast as one of the call types. rpc.go and rpc_test.go are renamed to match.
The proto method option that selects a plain remote call is named rpc, so a reader of gorums.proto sees rpc next to quorumcall, multicast, and unicast, none of which repeat the C in RPC. It becomes remotecall, matching the RemoteCall function it selects and the generated file it names. The option is set by the generator rather than by hand, which the declaration now says.
The envelope holds a decoded protobuf message alongside the wire message it came in, and both were reachable as Msg: the field, and the embedded stream.Message. Proto names the field for what it holds and removes the collision with the embedded type's own name.
ClientCtx was the last abbreviated type in the public API, and it is not client-specific: it carries the context and state of a call, and an interceptor receives it on both the quorum and one-way paths. CallContext names that. QuorumInterceptor becomes ClientInterceptor to match, since it now applies to any client-side call, not only quorum calls. Files are renamed so that a reader can find a declaration from its subject: mgr.go -> outbound_manager.go opts.go -> dial_options.go config_opts.go -> node_source.go handler.go -> server_handler.go client_interceptor.go -> call_context.go with the matching test files, plus async_test.go, correctable_test.go, and quorumcall_test.go grouped under the call_ prefix they all exercise, and opts_test.go widened to options_test.go.
migration.md becomes migration-v2-to-v3.md. Its title already scoped it to the v2-to-v3 iterator migration, but the filename read as though it were the only migration guide, which it is not. design-doc-layering.md is removed. It is a research note that predates the current typed interceptor model rather than maintained documentation, so it belongs in the private work area, where it is already tracked, alongside the other deferred proposals.
Generated output only, produced by make genproto. The type aliases injected into every generated file follow the Config rename, handler signatures follow the ServerContext rename, and the bundled static template follows both. The recorded protoc version also advances, since these files were last generated with an older release.
Config.SortBy became Sort and the comparators gained a By prefix, but the doc comments on ByID and ByLatency kept their old subjects. A Go doc comment must open with the name it documents, so as written neither rendered as documentation for its var.
The identity MapRequest transform names a node it never reads. Naming it _ says the parameter is deliberately ignored, which is what the two neighbouring benchmark cases already do.
Five headings under "Interceptors for Request/Response Transformation" called their subjects ServerInterceptors. MapRequest, MapResponse and the three worked examples all take a CallContext and wrap a ResponseSeq, so they are ClientInterceptors; the server-side section below them is unaffected. The StorageServer interface and its worked implementation also gave ReadCorrectable a send callback returning an error, and an error return of its own. Generated streaming-correctable methods take func(*Response) and return nothing, so neither was implementable as written.
meling
force-pushed
the
gorums/api-renames
branch
from
August 12, 2026 12:38
37fb53f to
200352f
Compare
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
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.
One vocabulary change per commit, so each can be read on its own. Thirteen commits, twelve of them mechanical.
Systemis folded intoServer.Systemwrapped aServer, a listener, and a list of closers; every method either delegated or managed the listener, so a user had two types, twoStopmethods, and two places to look for the peer configuration.Servernow owns its listener:WithAddrrecords the address,ListenAndServebinds it,Serveaccepts an external one,Addrreports whichever is in effect, andStopcloses it.NewLocalServersreplacesNewLocalSystems. Services register directly on theServer, which is what the generated registration functions already take.The renames:
ConfigurationConfigConfigContextandNewConfig, which already used the short formServerCtxServerContextConfigContextandNodeContextClientCtxCallContextQuorumInterceptorClientInterceptorNodeListOptionNodeSourceRPCCallRemoteCallWithDialOptionsWithGRPCDialOptionsWithInterceptorsWithServerInterceptorsMessage.MsgMessage.Protostream.MessageQuorumCallError.NodeErrorsNumErrorsConfig.SortByConfig.Sortslices.SortFuncID,Latency,LastNodeErrorByID,ByLatency,ByLastErrorNodemethods they readThe
rpcproto method option becomesremotecallto match, and files are renamed so a declaration can be found from its subject:mgr.gotooutbound_manager.go,opts.gotodial_options.go,config_opts.gotonode_source.go,handler.gotoserver_handler.go.Configjoins the reserved identifiers injected into generated code, so a proto message namedConfignow collides where one namedConfigurationused to; two test protos are adjusted.Generated output is the last commit, per the repository rule.
Verification:
go test ./... -count=2,go test -C examples ./...,go vet -tags=integration ./...,gofmt -l.