Skip to content
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

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

Merged
merged 2 commits into from
Jun 1, 2024

Conversation

NadavShmayo
Copy link
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
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
65 checks passed
blinkbear pushed a commit to blinkbear/vllm that referenced this pull request Jun 3, 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
Temirulan pushed a commit to Temirulan/vllm-whisper that referenced this pull request Sep 6, 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