fix(resources): honor caller-supplied imageName in Live* resources#339
Open
deanq wants to merge 2 commits into
Open
fix(resources): honor caller-supplied imageName in Live* resources#339deanq wants to merge 2 commits into
deanq wants to merge 2 commits into
Conversation
LiveServerlessMixin and the Live*/CpuLive* subclasses previously locked imageName to the Flash runtime image: - The imageName property setter was a no-op (silently dropped user input). - The @model_validator(mode="before") hooks unconditionally rewrote data["imageName"] to get_image_name(<type>, python_version). Together they made Endpoint(image=...) client-mode unreachable under FLASH_IS_LIVE_PROVISIONING=true: the user's image was discarded and Flash's wrapper was deployed instead, returning empty envelopes with no warning. Treat the Flash runtime image as a *default*, not a *lock*: - _apply_default_live_image() only writes data["imageName"] when the caller did not supply one. - Drop the imageName property override on the mixin; reads/writes go through Pydantic's normal field machinery so model_dump, drift hashes, and setattr stay consistent. - Endpoint.__init__ logs an info line when the caller supplies image= so the substitution choice is observable. Update three existing tests that asserted the old "locked" behavior to cover both the default-fallback path and the BYO-image override path across all four Live* classes. Refs AE-3153
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Live* serverless resource handling so that a caller-supplied imageName is honored under live provisioning, instead of being silently overwritten by the Flash runtime image. This restores Endpoint(image=...) client-mode behavior when FLASH_IS_LIVE_PROVISIONING=true while keeping the Flash runtime image as the default for decorator-mode usage.
Changes:
- Updated Live* resource model validators to only default
imageNamewhen the caller didn’t supply one, and removed the no-opimageNameproperty override so Pydantic field behavior is preserved. - Added an info log in
Endpoint.__init__whenimage=is provided to make image selection observable. - Updated/added unit and integration tests to cover both default and override
imageNamepaths for LiveServerless/CpuLiveServerless/LiveLoadBalancer/CpuLiveLoadBalancer.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/resources/test_live_serverless.py | Updates unit tests to validate imageName defaulting vs override for Live* resources. |
| tests/integration/test_lb_remote_execution.py | Adjusts integration coverage to verify LB Live* default image plus override behavior. |
| tests/integration/test_cpu_disk_sizing.py | Updates integration tests to reflect default vs override semantics for LiveServerless variants. |
| src/runpod_flash/endpoint.py | Logs when a user-supplied image= is provided to Endpoint. |
| src/runpod_flash/core/resources/live_serverless.py | Changes Live* defaulting logic to honor caller imageName and removes the “locked image” property override. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review on PR #339: - Guard _apply_default_live_image against non-dict input in @model_validator(mode="before"), matching network_volume.py:109 pattern. Prevents AttributeError when Pydantic passes a model instance during revalidation or nested construction. - Update test docstrings to reflect "default unless overridden" semantics (replacing stale "locked image" wording). - Add regression test exercising LiveServerless.model_validate on a model instance to cover the guard path.
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.
Summary
LiveServerlessMixinand theLive*/CpuLive*subclasses silently swallowed caller-suppliedimageName: the setter was a no-op and the@model_validator(mode="before")hooks unconditionally rewrotedata["imageName"]to the Flash runtime image. This madeEndpoint(image=...)client-mode unreachable underFLASH_IS_LIVE_PROVISIONING=true— user image was discarded, Flash's wrapper deployed instead, jobs returned empty envelopes.data["imageName"]when the caller didn't supply one. Drop the property override so reads/writes use Pydantic's normal field machinery (keepsmodel_dump, drift hashes, andsetattrconsistent).Endpoint.__init__now logs an info line when the caller passesimage=so the substitution choice is observable.Test plan
make quality-checkpasses (2696 unit + 53 integration tests, coverage 85.4%).Live*classes:imageName-> Flash runtime imageimageName="custom/..."-> stored verbatimLiveServerless(name=...)(no image) still resolves torunpod/flash:py3.12-*.Endpoint(name=..., image="runpod/worker-v1-vllm:v2.18.1", gpu=...)underFLASH_IS_LIVE_PROVISIONING=trueprovisions a template whoseimageNamematches the supplied value.