Skip to content

Conversation

@lawrencelai
Copy link
Collaborator

core.model.run_model_container method, parameter "resources" define as dict. I found auto_deployed.py wrongly pass the Object (DeployResources). To fix this, I convert the object to dict and pass it into the method

…s dict. I found auto_deployed wrongly pass the Object (DeployResources). To fix this, I convert the object to dict and pass it into the method
@lawrencelai lawrencelai self-assigned this Jan 16, 2026
@lawrencelai lawrencelai added the bug Something isn't working label Jan 16, 2026
Copy link
Member

@phoevos phoevos left a comment

Choose a reason for hiding this comment

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

Hey @lawrencelai, thanks for looking into this.

Please take a look at my comment and make sure that all tests pass.
Apart from that, note that we require signed commits for merging to main; please look into configuring git with your GPG key and using it to sign your commits before pushing to GH: https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits

deployment_type=ModelDeploymentType.AUTO,
ttl=model_config.idle_ttl,
resources=model_config.deploy.resources if model_config.deploy else None,
resources=model_config.deploy.__dict__ if model_config.deploy else None,
Copy link
Member

Choose a reason for hiding this comment

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

Any reason for not using pydantic's built-in model_dump here?

We generally tend to avoid calling internal/magic/dunder methods like that directly. __dict__ is used to fetch an object's writable attributes, not convert it to a dictionary. Consider the following example:

deploy = DeploySpec(resources=DeployResources(limits=ResourceLimits(memory="4g")))

Calling deploy.__dict__ will return the following, which isn't really what you want (notice how it doesn't work recursively):

{'resources': DeployResources(limits=ResourceLimits(memory='4g', cpus=None), reservations=None)}

Instead, you should be calling model_dump which is build for this exact purpose, replacing the now deprecated dict method of pydantic's BaseModel.

Suggested change
resources=model_config.deploy.__dict__ if model_config.deploy else None,
resources=model_config.deploy.model_dump() if model_config.deploy else None,

The above will give you the intended result:

{'resources': {'limits': {'memory': '4g', 'cpus': None}, 'reservations': None}}

@phoevos
Copy link
Member

phoevos commented Jan 16, 2026

I would also recommend updating the PR title to something more descriptive of the actual change/fix rather than a vague reference to the error that exposed the bug! Following the conventions used in this repo, I would go for something along the lines of fix: Convert pydantic model to dict for run_model_container

@lawrencelai lawrencelai changed the title PR Fix Issue #6 -cmg: Auto deploy throws "Failed to deploy model" fix: Convert Pydantic model to dict for run_model_container Jan 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants