-
Notifications
You must be signed in to change notification settings - Fork 164
fix: convert image string to ImageRow object on model service creation #5633
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a type mismatch issue in model service creation where a string image identifier was being passed instead of the expected ImageRef object. The fix implements proper image resolution to convert the string to an ImageRef object while maintaining backward compatibility for JSON serialization.
Key changes:
- Added image resolution logic using
ImageRow.resolve()
to convert image string toImageRef
object - Updated
kernel_config.image_ref
to use the resolvedImageRef
instead of raw string - Maintained string format for
endpoint_resource_opts.image_ref
to preserve JSON serialization compatibility
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
image_row = await ImageRow.resolve( | ||
session, | ||
[ | ||
ImageIdentifier(model_spec.image, model_spec.architecture), | ||
ImageAlias(model_spec.image), | ||
], | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ImageRow.resolve() call lacks error handling. If image resolution fails (e.g., image not found), this could raise an unhandled exception. Consider wrapping this in a try-catch block and providing a meaningful error message for debugging model service creation failures.
image_row = await ImageRow.resolve( | |
session, | |
[ | |
ImageIdentifier(model_spec.image, model_spec.architecture), | |
ImageAlias(model_spec.image), | |
], | |
) | |
try: | |
image_row = await ImageRow.resolve( | |
session, | |
[ | |
ImageIdentifier(model_spec.image, model_spec.architecture), | |
ImageAlias(model_spec.image), | |
], | |
) | |
except Exception as exc: | |
log.error( | |
"Failed to resolve image for model service creation: image='{}', architecture='{}'. Exception: {}", | |
model_spec.image, | |
model_spec.architecture, | |
exc, | |
) | |
raise ServiceInfoRetrievalFailed( | |
f"Failed to resolve image '{model_spec.image}' (architecture: '{model_spec.architecture}') for model service creation." | |
) from exc |
Copilot uses AI. Check for mistakes.
8ec0d91
to
8b02f20
Compare
resolves #5633 (BA-MMM)
The fix addresses an issue in src/ai/backend/manager/sokovan/deployment/deployment_controller.py:517 where model service creation was failing due to improper image handling:
The change ensures model services can be created successfully by providing the kernel configuration with the expected ImageRef object type while maintaining backward compatibility for stored endpoint resource options.
Checklist: (if applicable)
ai.backend.test
docs
directory