Revert logging file name modification

[Kludex]

Reported on b48d32d#commitcomment-76948401.

The problem is that users are relying on configuration files like that one. _logging.py shouldn't be private module if they are using it. Also, I do think it's meant for users to do that. 🤔

Thoughts here @euri10 ?

[euri10]

yep, I'd prefer we add a note to say that change will crash the custom logger, it crashed mine too, just need to change the name inside the custom yaml config from logging to _logging

[Kludex]

But then... We are making the user access a private module (even if not directly), and motivating them to do it? 🤔

[euri10]

But then... We are making the user access a private module (even if not directly), and motivating them to do it? thinking

fair enough, e could also put the formatters in a public uvicorn.formatters and let the _logging module private ?

[florimondmanca]

Yes, let's keep a publicly-accessible place for the logging helpers we expose to users. I stumbled upon this myself trying to copy this over... #491 (comment) after upgrading to 0.18.1.

For minimum disruption, I would say we could:

1. Add uvicorn.formatters.*
2. Add uvicorn.logging.*, with a module-level DeprecationWarning with instruction to switch to uvicorn.formatters.*.

It's also possible #1426 wasn't a good idea in the first place, and this PR is legit in the current state. In what situations was the name uvicorn.logging clashing with the stdlib logging?

In all cases, I think we should add a test that imports the items meant for public logging API -- the ones that would end up in strings in logging configuration files.

[tjader]

Naming the module uvicorn.formatters is more ambiguous thant uvicorn.logging in the sense that there are many things that can be formatted. If you pick up a Formatter from the uvicorn.logging module, you know what you're looking for.
I hope you don't go for implicitly hidden module name uvicorn._logging.
Also, the only way you will end up having a name clash with the standard module is if you have your current working directory inside the (installed) path of uvicorn, or if you have done something non-standard with your PYTHONPATH. Any other properly installation of uvicorn shouldn't have this issue.

[florimondmanca]

Based on this discussion I'm also in favor of straight-up reverting #1426. 👍

Adding a small "try to import logging stuff" test case would make this PR even better, but IMO it's acceptable in its current state.

[macieyng]

I'm no expert, but changing import logging to import logging as py_logging in uvicorn internally would be sufficient to avoid name clashing while preserving uvicorn.logging. WDYT?

[florimondmanca]

IIUC doing import logging when working on Uvicorn, i.e. with the project root directory as the cwd, would import the std logging library.

You'd need to do from uvicorn import logging or from ..logging import ... to access Uvicorn's logging module, which is what we probably do?

That's why I was confused and asked in what cases the name clashing which motivated #1426 would occur.

[euri10]

iirc some people were running python uvicorn/main.py but I'm not too sure now

[Kludex]

Based on this discussion I'm also in favor of straight-up reverting #1426. 👍

Adding a small "try to import logging stuff" test case would make this PR even better, but IMO it's acceptable in its current state.

I didn't get what test you want. Can you rephrase/exemplify?

[ant31]

@Kludex Ith ink it's about to add a quick test that uses "uvicorn.logging".
So in the case of future PR similar to 1426, modifying an important end-user interface this test would fail.

[florimondmanca]

I didn't get what test you want. Can you rephrase/exemplify?

Something like this:

def test_logging_api() -> None:
    # These items may be referenced by users in logging configurations.
    from uvicorn.logging import DefaultFormatter  # noqa
    from uvicorn.logging import AccessFormatter  # noqa

[polyrand]

The logging -> _logging rename also broke some of my apps when updating to 0.18.0. I never had issues with uvicorn.logging. Using _logging may be confusing for some users, since it seems you're accessing a private module and not doing the right thing.

I think it may be worth mentioning submodule renames in the release notes, too.

[Kludex]

I'll make sure to mention those if it happens again. Thanks @polyrand :)

[polyrand]

I'll make sure to mention those if it happens again. Thanks @polyrand :)

Thank you all for the awesome work ❤️