fix(serve): remove IC data-source collapse hack from recommendation deploy#6101
fix(serve): remove IC data-source collapse hack from recommendation deploy#6101ZealSV wants to merge 1 commit into
Conversation
…eploy Inference Components now support AdditionalModelDataSources natively (kernel tuning / speculative decoding channels), so _deploy_recommendation no longer needs to collapse an optimized recommendation's base_model + draft channels into a single primary ModelDataSource. Deploy the recommendation's ModelPackage directly and let the hosting stack resolve the channels. Removes the base_model-promotion / OPTION_SPECULATIVE_DRAFT_MODEL rewiring block and its now-unused shape imports (AdditionalModelDataSource, ModelDataSource, S3ModelDataSource, SPECULATIVE_DRAFT_MODEL). Updates the speculative-decoding unit tests to assert the ModelPackage pass-through.
| { | ||
| "Image": "123.dkr.ecr.us-west-2.amazonaws.com/djl-inference:latest", | ||
| "Environment": {}, | ||
| "AdditionalModelDataSources": [ |
There was a problem hiding this comment.
Test signal (minor, non-blocking): this AdditionalModelDataSources fixture (base_model + draft_model) is inert. The rewritten _deploy_recommendation now reads only described["ModelApprovalStatus"] — it no longer inspects InferenceSpecification/Containers — so deleting these channels from the fixture wouldn't change any assertion, and this test drives the same production path as test_no_additional_sources_uses_model_package. Its only differential value is the two negative assertions (no model_data_source / no additional_model_data_sources), which only bite if collapse logic is re-introduced. Consider a one-line comment noting the sources are intentionally present as a regression guard against re-introducing client-side collapse, so the intent is explicit.
| # package; the hosting stack resolves those channels itself, so no | ||
| # client-side collapsing is needed. | ||
| container_def_kwargs = {"model_package_name": model_package_arn} | ||
| if inference_specification_name: |
There was a problem hiding this comment.
Test gap (minor, non-blocking): this is a behavior change — optimized/speculative-decoding recs now thread inference_specification_name into the container, whereas the old inline-container branch dropped it. No unit test asserts the container actually carries it: the deploy-path tests seed _rec_row(spec_name="A") (so the container would receive inference_specification_name="A") but only assert model_package_name. A regression that dropped this if inference_specification_name: branch would pass every existing test. Suggest adding assert container.inference_specification_name == "A" in test_optimized_recommendation_deploys_via_model_package to lock it in.
2 participants
Inference Components now support AdditionalModelDataSources natively (kernel tuning / speculative decoding channels), so _deploy_recommendation no longer needs to collapse an optimized recommendation's base_model + draft channels into a single primary ModelDataSource. Deploy the recommendation's ModelPackage directly and let the hosting stack resolve the channels.
Removes the base_model-promotion / OPTION_SPECULATIVE_DRAFT_MODEL rewiring block and its now-unused shape imports (AdditionalModelDataSource, ModelDataSource, S3ModelDataSource, SPECULATIVE_DRAFT_MODEL). Updates the speculative-decoding unit tests to assert the ModelPackage pass-through.
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.