Skip to content

simplify config#171

Merged
nirrozenbaum merged 2 commits intollm-d:mainfrom
nirrozenbaum:config-change
Jun 10, 2025
Merged

simplify config#171
nirrozenbaum merged 2 commits intollm-d:mainfrom
nirrozenbaum:config-change

Conversation

@nirrozenbaum
Copy link
Collaborator

@nirrozenbaum nirrozenbaum commented Jun 3, 2025

This PR simplifies the code in pkg/config in the following manner:

  • current code in main is calling NewConfig (which returns *Config) and then config.LoadConfig(), which overrides the default values that were filled in the NewConfig func.
    following best practices, a common practice (instead of the existing logic) is to have instead a LoadConfig function that returns *Config and remove the NewConfig function completely.
  • additional (minor) change is to keep all pluginNames in a slice once and then reuse it when initializing prefil/decode (instead of the existing code that duplicates the list twice).

Signed-off-by: Nir Rozenbaum <nirro@il.ibm.com>
Signed-off-by: Nir Rozenbaum <nirro@il.ibm.com>
Copy link
Member

@vMaroon vMaroon left a comment

Choose a reason for hiding this comment

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

LGTM

@nirrozenbaum nirrozenbaum merged commit 47ac73d into llm-d:main Jun 10, 2025
2 checks passed
@nirrozenbaum nirrozenbaum deleted the config-change branch June 10, 2025 14:46
mayabar pushed a commit to mayabar/llm-d-inference-scheduler that referenced this pull request Jun 15, 2025
* simplify config

Signed-off-by: Nir Rozenbaum <nirro@il.ibm.com>

* addressed code review comment from mayabar

Signed-off-by: Nir Rozenbaum <nirro@il.ibm.com>

---------

Signed-off-by: Nir Rozenbaum <nirro@il.ibm.com>
pierDipi pushed a commit to pierDipi/llm-d-inference-scheduler that referenced this pull request Nov 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants