[HWORKS-3212] Deployment schema: validated inference requests, default predictor, and feature view deployments - #648
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new DefaultPredict subclass examples omit the serving-wrapper handoff (run_kserve_wrapper()), which can leave readers with a non-runnable script on backends that execute the file directly.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds new Hopsworks documentation describing “deployment schema” (validated inference request/response contracts), the default predictor flow, and how to deploy a Feature View directly, and then links these concepts from existing serving/registry docs and navigation.
Changes:
- Add a new Deployment Schema guide covering schema inference, validation, discovery, revisions/rollback, errors, and logging/monitoring semantics.
- Add a new Feature View deployment guide explaining how to serve transformed feature vectors as an endpoint.
- Add navigation entries and cross-links from serving index, predictor, deployment, REST API, and model schema pages.
File summaries
| File | Description |
|---|---|
| mkdocs.yml | Adds nav entries for the two new guides. |
| docs/user_guides/mlops/serving/rest-api.md | Documents schema discovery endpoint on the Hopsworks REST API and links to the new guide. |
| docs/user_guides/mlops/serving/predictor.md | Introduces default predictor section and links to deployment schema guide; clarifies predictor script validation behavior. |
| docs/user_guides/mlops/serving/index.md | Adds pointers to deployment schema/default predictor and feature view deployment. |
| docs/user_guides/mlops/serving/deployment.md | Adds “no predictor script” tip and notes where schema documents are stored for revisions. |
| docs/user_guides/mlops/serving/deployment_schema.md | New comprehensive guide for deployment schema + default predictor behavior and contracts. |
| docs/user_guides/mlops/registry/model_schema.md | Adds clarification about model schema vs deployment schema and links to the new guide. |
| docs/user_guides/fs/feature_view/deployment.md | New guide for deploying a feature view as an online endpoint (feature vectors, logging, monitoring, access). |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟢 Approval recommended
The changes are documentation-only, integrate cleanly into navigation, and the new cross-references and examples are consistent with existing docs structure.
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
557e75a adds complete deployment-creation examples to the deployment guide: a model registered with its feature view (register, deploy with passed features, start, read the schema, predict) and a feature view deployed without a model, each pointing to its guide. It also states that the hand-over check covers local scripts only. |
There was a problem hiding this comment.
🟡 Changes recommended
A few documentation/codebase convention issues remain (notably “Feature View” capitalization consistency and the underscore filename in deployment_schema.md) that should be aligned before publishing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
Previously missed (2) — in code that hasn't changed since the last review.
docs/user_guides/mlops/registry/model_schema.md:15
- Terminology: this new info box uses "feature view" in prose; the docs guidelines prefer "Feature View" as a proper noun.
docs/user_guides/mlops/serving/predictor.md:168 - Terminology: this new section refers to "feature view" in prose; the docs guidelines prefer "Feature View" as a proper noun.
docs/user_guides/mlops/serving/deployment_schema.md:17
- Terminology: this intro sentence uses "feature view" in the link text; the docs guidelines prefer "Feature View" as a proper noun.
A [feature view can be deployed on its own][feature-view-deployment] with the same class and the same contract, returning the transformed feature vector instead of a prediction.
docs/user_guides/mlops/serving/deployment.md:199
- Terminology: this new section refers to "feature view" in prose; the docs guidelines prefer "Feature View" as a proper noun.
A feature view deploys on its own and returns the transformed feature vectors, for callers that run the model elsewhere.
The deployment pins the training dataset whose statistics the transformations use.
- Files reviewed: 8/8 changed files
- Comments generated: 6
- Review effort level: Lite
|
Copilot round 4: the filename is now |
There was a problem hiding this comment.
🟢 Approval recommended
Changes are documentation-only, nav is updated for the new pages, and no broken references or style violations were found in the modified hunks.
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
5bf2e18 to
a019438
Compare
|
External review round 3, addressed in 7f31910: the schema guide now says the batch limit is part of the schema (changing |
9c0fab4 to
58e8d06
Compare
|
58e8d06 (squashed) adds the section "Deployments without lookups" to the schema guide: every stored feature passed (no serving keys; the view only computes on-demand features and transforms) and a model without a feature view ( |
58e8d06 to
90469c2
Compare
|
Follow-up on the ModelSchema examples: the model schema guide now marks Strict build and markdownlint green. Head 90469c2. |
90469c2 to
3d1cb69
Compare
|
Feature logging is now described as asynchronous only (owner decision): the two-mode paragraph, the |
3d1cb69 to
c010635
Compare
a7eac7e to
30534ba
Compare
P1: Limit the pod-side validation claim to REST V1The guide says every request carrying a deployment schema is validated in the pod before predictor code runs. The docker-images implementation intentionally skips schema enforcement for every protocol other than KServe REST V1, including gRPC, and only logs a warning. Client-side validation is also REST-only. A custom-script gRPC deployment can therefore accept an invalid request despite this documentation. State that enforcement currently covers REST V1 only, or reject schemas on unsupported protocols. P2: Document that the feature-view Istio URL may be unavailable
|
…t predictor, and feature view deployments https://hopsworks.atlassian.net/browse/HWORKS-3212 A Hopsworks deployment has no declared request contract. Callers learn what a deployment expects from its predictor script, malformed rows fail inside user code or the online store lookup with opaque errors, every model served from a feature view repeats the same lookup, transform, predict and log boilerplate, and a feature view cannot be served on its own. Infer a deployment schema from the feature view (serving keys, passed features, request parameters of on-demand features, response shape), publish it as an immutable content-addressed document bound to the deployment revision, validate requests against it in the client and in the serving pod, and serve model and feature view deployments without a script through a default predictor. The client library carries the schema, the default predictor and the deploy API (hopsworks-api); the backend exposes the published schema and starts model-less deployments (hopsworks-ee); the serving image enforces the schema before user code and bounds the async feature logger (docker-images); the docs and the loadtest describe and exercise the flow. The docs add a guide for the deployment schema (inference, discovery, revisions, errors, feature logging and monitoring, environment variables) and one for deploying a feature view, and point the predictor, deployment, REST API, serving index and model schema pages at them. Enforcement in the pod needs an inference image with the updated wrapper, which the guide states. Feature logging is described as asynchronous only: no logging mode, no LOGGING_FAILED code. The API reference entries for DeploymentSchema, DefaultPredict and FeatureView.deploy are plain code rather than cross-references, because the docs build installs the client from its main branch, where those objects do not exist until hopsworks-api#1147 merges; they become links in a follow-up once it has. The schema guide also covers deployments without lookups: every stored feature passed, and a model without a feature view deployed with passed_features naming its input columns. The model schema guide marks ModelSchema as deprecated, since a model registered with a feature view takes its schema from the feature view's training dataset, and the deployment examples no longer pass one. Request parameter types are explained as coming from the transformation function's argument annotations, with unannotated arguments unresolved. Three corrections. Pod-side and client-side validation cover REST V1 only, which the guide now says instead of claiming every request is validated; gRPC is served unchecked. The feature view guide says that get_inference_url() returns None when the Istio ingress is not configured, and points at the Hopsworks REST path instead. The logging buffers are described as bounded by rows, matching the client. Reviewed-by: GitHub Copilot <Copilot@users.noreply.github.com> Signed-off-by: Jim Dowling <jim@logicalclocks.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
30534ba to
9b680a7
Compare
|
Both corrected in 9b680a7. P1, the validation claim. The introduction said every request is validated in the pod. It now says every REST V1 request, with the next sentence stating that enforcement covers the KServe REST V1 protocol only, that a gRPC deployment is served without it, and that the pod logs a warning at startup. The custom-script paragraph carries the same qualification and adds that the client validates REST requests only, so a gRPC client is unchecked on both sides. That matches Rejecting schemas on unsupported protocols is the stronger option and I did not take it here: it would turn an existing gRPC deployment into a startup failure on upgrade. Worth deciding separately, and I have noted it with the other deferred items on the specification page rather than changing behaviour in a documentation pull request. P2, the feature view Istio URL. The feature view guide now says Also in this push, since it touches the same guide: the feature logging buffers are documented as bounded by rows rather than by requests, matching the client change in hopsworks-api#1147, and the environment variable table says a value that is not a positive integer is ignored. markdownlint is clean on both files. |
…io into HWORKS-3212
086a3d0 to
156d3d3
Compare
https://hopsworks.atlassian.net/browse/HWORKS-3212 The guide said the buffers hold at most FEATURE_LOGGER_QUEUE_SIZE rows, but an empty backlog admits one request whatever its size, so a deployment whose batches are larger than its buffer logs instead of dropping every request. The peak is then that single batch, itself capped by the schema's batch limit. Say so rather than promising a flat cap the code does not enforce. Signed-off-by: Jim Dowling <jim@logicalclocks.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
156d3d3 to
b00f98a
Compare
JIRA: https://hopsworks.atlassian.net/browse/HWORKS-3212
Summary
New guide
user_guides/mlops/serving/deployment_schema.md: inference, discovery endpoint, revisions and rollback, error codes andrequest_id, feature logging and monitoring, environment variables (SERVING_*,FEATURE_LOGGER_QUEUE_SIZE).New guide
user_guides/fs/feature_view/deployment.md: deploying a feature view directly.Pointers from the predictor, deployment, REST API, serving index and model schema pages; nav entries in
mkdocs.yml.ModelSchemamarked deprecated in the model schema guide; the deployment examples no longer passmodel_schema=, and the deployment schema guide's model-without-a-feature-view section usespassed_features=.Feature logging is described as asynchronous only: no logging mode, no
LOGGING_FAILEDcode, noSERVING_FEATURE_LOGGING_MODEenv var.The deployment schema guide explains that request parameter types come from the transformation function's argument annotations, and that an unannotated argument is unresolved until the schema is refined.
Review round (@dhananjay-mk): pod-side and client-side validation are described as REST V1 only (gRPC is served unchecked),
get_inference_url()is documented as possiblyNonefor a feature view deployment, and the logging buffers are described as bounded by rows.Related PRs
Test plan
mkdocs build -s(strict) with the client PR's SDK installed: exit 0.CI's
test-docs-buildbuilds the API reference fromlogicalclocks/hopsworks-api@main, wherehsfs.feature_view.FeatureView.deploy,hsml.default_predictor.DefaultPredictandhsml.deployment_schema.DeploymentSchemado not exist until hopsworks-api#1147 merges; the three API reference entries for them are plain code for now so the strict build passes, and become cross-references in a follow-up once the client has merged.markdownlint-cli2: 0 issues.hopsworks-docs snakeoil: no diff.Code samples match what the loadtest of this ticket runs on
jim-deployment-api(jobloadtest-3212-final7, 14 passed, 3 skipped in 7m43s, no teardown errors. The skips are the custom-script enforcement case and the two cases that read the feature log back: all three need the wrapper of the docker-images PR, and the cluster's stock inference image predates it (its logger sends events the sidecar rejects)).The deployment guide's Step 3b and 3c show creating a model deployment with its feature view and a feature view deployment without a model; the API reference examples live in the hopsworks-api PR.
External review round 3 (five findings: dual-key envelopes, unloaded-schema publication, revision-pinned enforcer role, batch limit in the contract, arrays collapsed to null) is folded into the single commit; the run above is on that commit.
Deployments without lookups (added at the owner's request): every stored feature passed (schema without serving keys; the view only computes on-demand features and transforms) and a model registered without a feature view (
default_predictor=Truewithpassed_features=naming its input columns in the model's order; a legacy columnar model schema still supplies them when present, andModelSchemais now deprecated). Loadtest casestest_all_features_passed_needs_no_lookupandtest_model_without_feature_viewpass on the cluster.Copilot review: 5 rounds applied as commits, then squashed to one commit per repo; every thread is resolved, none deferred. Declined with rationale on the threads: capitalising "feature view" in the docs,
verify=Falsein the loadtest.🤖 Generated with Claude Code
https://claude.ai/code/session_01UfFAHbtiLmDDSkdbWcXWCj