[BugFix] Ensure EngineArgs.create_engine_config is idempotent
#28515
Conversation
Signed-off-by: Nick Hill <nhill@redhat.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a bug where EngineArgs.create_engine_config was not idempotent, as it was mutating self.compilation_config. This mutation caused issues when the method was called multiple times, for instance, when setting up multiple API servers. The fix correctly resolves this by creating a deep copy of self.compilation_config and applying overrides to this copy, leaving the original object unmodified. This ensures the method can be called repeatedly without side effects. The change is correct and effectively solves the problem.
njhill
changed the title
EngineArgs.create_engine_config can be called >1 timeEngineArgs.create_engine_config is idempotent
|
Should we add a test for this? |
|
@DarkLight1337 I was thinking about a test but a basic test wouldn't have caught this particular bug... it happened to apply only to these specific 3-4 args if they were specified in a standalone way. |
|
Alright let's just merge this then |
…-project#28515) Signed-off-by: Nick Hill <nhill@redhat.com> Signed-off-by: Elizabeth Thomas <email2eliza@gmail.com>
…-project#28515) Signed-off-by: Nick Hill <nhill@redhat.com> Signed-off-by: George D. Torres <gdavtor@gmail.com>
Signed-off-by: Nick Hill <nhill@redhat.com> (cherry picked from commit 327c0a9)
2 participants
Issue introduced in #26016.
This was in particular breaking
--api-server-count> 1 with cudagraph size related args.Thanks to @elvischenv for finding this!