Skip to content

Resolve staged sklearn model path on the worker in RunInference xlang tests - #40795

Open
tarun-google wants to merge 5 commits into
apache:masterfrom
tarun-google:runinference_staged_model_path
Open

tarun-google wants to merge 5 commits into
apache:masterfrom
tarun-google:runinference_staged_model_path

Conversation

@tarun-google

@tarun-google tarun-google commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

The Java and Go RunInference cross-language tests hardcode where the Python SDK
harness puts staged files (<semiPersistDir>/staged/sklearn_model, or
/tmp/staged/sklearn_model in Go). That location belongs to the container's
boot program and is only known on the worker, so the tests break when it
changes (see #40699, which moves Python staging to
<semi_persist_dir>/<worker id>/staged).

The test expansion service (expansion_service_test.py) attaches the model to
the Python environment as a staged file named sklearn_model. The tests now
pass that name and use a small SklearnModelHandlerNumpy subclass that builds
the full path from SEMI_PERSISTENT_DIRECTORY in load_model(). load_model()
runs on the worker after boot has set that variable. The subclass uses the
public update_model_path(), can be called repeatedly, and raises a KeyError
naming the variable if it is missing.

Test-only change. It works with both the current and the per-worker staging
layout, so it can merge ahead of #40699.

Testing (DirectRunner + Python expansion service, Python 3.11):
./gradlew :sdks:python:test-suites:direct:xlang:validatesCrossLanguageRunnerJavaUsingPython --tests '*RunInferenceTransformTest*' -PpythonVersion=3.11

@github-actions github-actions Bot added java go extensions size/s S: 10-49 lines changed labels Oct 26, 2026
@tarun-google
tarun-google marked this pull request as ready for review October 26, 2026 02:06

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📋 Review Summary

This PR updates the Java and Go cross-language RunInference tests to resolve the staged sklearn_model path at worker runtime via SEMI_PERSISTENT_DIRECTORY in load_model(), rather than hardcoding the staging directory at pipeline construction time. The implementation is clean, idempotent across multiple load_model() calls, and ensures compatibility with per-worker staging directory layouts (#40699).

🔍 General Feedback

  • Using os.path.basename(self._model_uri) combined with self.update_model_path(...) cleanly keeps load_model() idempotent if called multiple times.
  • Both the unkeyed and keyed (KeyedModelHandler) test paths in Java as well as the Go xlang integration test are properly covered.

💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.

@github-actions

Copy link
Copy Markdown
Contributor

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

extensions go java size/s S: 10-49 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant