refactor(api): consolidate proto files and trim EngineError - #29
Closed
connorcarpenter15 wants to merge 1 commit into
Closed
refactor(api): consolidate proto files and trim EngineError#29connorcarpenter15 wants to merge 1 commit into
connorcarpenter15 wants to merge 1 commit into
Conversation
Reduce the openengine/v1 file set from 10 to 8 by folding two small files into the domain file that owns their concern, and remove dead message/field surface. Package, message names, and field numbers are unchanged -- wire- and (whole-package) codegen-compatible. - Fold observability.proto into server.proto. Post-cleanup it held only load reporting (GetLoadRequest/LoadInfo/RankLoadInfo); "observability" was a misnomer. server.proto now covers static identity/capacity plus dynamic load -- everything the server reports about itself. - Fold input.proto into generation.proto, its sole remaining consumer, and delete the dead task-input types (TaskInput, MultimodalTaskInput, TaskInputType) orphaned when the non-generative task RPCs were removed. TokenIds/Modality/MediaItem move as-is. - Trim EngineError: drop `details` (never set or read by any engine/sidecar) and `retry_after_ms` (only ever absent), and the now-unused struct import. Keep `retryable` -- servers populate it (SGLang computes it from a 503) even though the sidecar does not yet read it. Sync docs/api.md and the proto README table. Validated with protoc and markdownlint-cli2. Signed-off-by: Connor Carpenter <connorc@nvidia.com>
Member
Author
|
Folded into #28 — keeping the Drain removal and the proto consolidation / EngineError trim in a single PR. |
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
Consolidates the
openengine/v1proto file set from 10 → 8 and trims dead message/field surface. Package name, message names, and field numbers are unchanged, so this is wire-compatible and (for whole-package codegen) transparent to consumers — only which file a message lives in changes.Consolidation
observability.proto→server.proto. After earlier cleanups,observability.protoheld only load reporting (GetLoadRequest,LoadInfo,RankLoadInfo) — "observability" had become a misnomer. Load is the dynamic half of whatserver.protoalready reports (static identity/capacity/roles), so they belong together: "everything the server reports about itself."input.proto→generation.proto. After the non-generative task RPCs were removed,generation.protowasinput.proto's only consumer.TokenIds/Modality/MediaItemmove in as-is.Dead-code removal
TaskInput,MultimodalTaskInput,TaskInputTypewere orphaned when Embed/Classify/Score were removed (nothing referenced them). Deleted.EngineErrorfields — droppeddetails(never set or read by any engine or the sidecar) andretry_after_ms(only ever absent), plus the now-unusedstructimport. Keptretryable: all three servers populate it (SGLang computes it from a503), even though the sidecar does not yet read it — removing it would discard a real signal rather than dead code.Result
10 → 8 files:
openengine·generation(+input) ·kv·model·server(+observability/load) ·lifecycle·lora·error.Validation
protoc -I proto --experimental_allow_proto3_optionalbuilds the full descriptor set clean.markdownlint-cli2clean on all Markdown.Notes
mainand touchopenengine.proto/docs/api.md; whichever merges first, the other needs a trivial rebase. This PR deliberately leaves the errors-section drain/runtime-event prose to refactor(api): remove Drain RPC, consolidate proto files, trim EngineError #28 to avoid overlapping edits.EngineError.details/retry_after_ms; no reader exists, so nothing breaks functionally.retryablehandling is unchanged (still populated; wire the sidecar to honor it if that signal should matter).Compatibility
No wire-format change from the consolidation (same package/messages/field numbers). The
EngineErrorfield removals are wire-breaking for those two fields only, which no consumer reads.