Skip to content

[Bugfix] Fix call to init_logger in openai server#4765

Merged
DarkLight1337 merged 2 commits intovllm-project:mainfrom
NadavShmayo:fix_openai_logger
Jun 1, 2024
Merged

[Bugfix] Fix call to init_logger in openai server#4765
DarkLight1337 merged 2 commits intovllm-project:mainfrom
NadavShmayo:fix_openai_logger

Conversation

@NadavShmayo
Copy link
Copy Markdown
Contributor

Currently when running the OpenAI entrypoint, all logs coming from this file will not be shown.
This is because the call to init_logger at the top of the file uses the __name__ global variable, and it's value is __main__ since this is the main file, which causes the init_logger function to return a logger with default configuration instead of the one configured in the logger file.

To solve this I changed the call to init_logger to pass a string instead of __name__, but another possible and potentially more elegant solution would be changing the init_logger function so the logger name is optional, and omitting it in the OpenAI entrypoint:

def init_logger(name: str = None) -> Logger:
    """The main purpose of this function is to ensure that loggers are
    retrieved in such a way that we can be sure the root vllm logger has
    already been configured."""

    return logging.getLogger(name or 'vllm')

@DarkLight1337
Copy link
Copy Markdown
Member

DarkLight1337 commented May 31, 2024

Sorry for keeping you waiting for so long.

I think that the preferred way to use OpenAI server is to use python -m <module> syntax, as evidenced by the docs. This should avoid the logging problem that is associated with executing the Python file directly.

Edit: Ok it seems that my understanding was wrong. In that case, let's adopt the first solution, since your second solution opens up the possibility of forgetting to pass in name to init_logger. Thanks for the contribution!

@DarkLight1337 DarkLight1337 enabled auto-merge (squash) May 31, 2024 15:51
@DarkLight1337 DarkLight1337 merged commit 37464a0 into vllm-project:main Jun 1, 2024
chengzhi-lu pushed a commit to chengzhi-lu/vllm-infersche that referenced this pull request Jun 3, 2024
robertgshaw2-redhat pushed a commit to neuralmagic/nm-vllm that referenced this pull request Jun 11, 2024
joerunde pushed a commit to joerunde/vllm that referenced this pull request Jun 17, 2024
xjpang pushed a commit to xjpang/vllm that referenced this pull request Jun 27, 2024
xjpang pushed a commit to xjpang/vllm that referenced this pull request Jul 8, 2024
xjpang pushed a commit to xjpang/vllm that referenced this pull request Jul 24, 2024
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.

2 participants