Maybe we can just uselogger as suggested in the linked Python docs? Just visually also seems to make sense to me calling methods on it like that:

Since it is a global I prefer theLOG. Also in the past I have usedlogger but then found myself accidentally typinglogging and not noticing what went wrong for awhile :(

Plus it is the way OpenStack does it:
https://github.com/openstack/ironic/blob/0659485d630b8651faa633f98b1802bdf244f186/ironic/drivers/modules/ipmitool.py#L61

As an example it is easy to do:

logging.debug("some message") when you really meant to dologger.debug("some message")

Which can cause problems. As I unfortunately know from my own experience.

Ok, it was just an idea as it is aLogger instance that we manipulate and not a constant asLOG would imply. I guesslog would also be somewhere in between.

What do you think of the nameLOGGER? For me it is the middle ground.

I mght be misremembering but thought I saw an earlier version of this PR where I liked the simplicity of just doing this in__init__.py a bit more - especially if this is extra module is being done just for testing.requests and a lot of others do that too just in__init__.py. Maybe we can look at other ways of testing if needed (or we can add more testing later)?

Added a docstring to explain why doing it here.

To me it just shifts the issue to imports if something happens to be imported e.g.import _clients would go before this. So I thought there might be a simple way for this, as I don't really see it done in popular python libraries. I know some useget_logger() helpers to lazily load loggers, maybe that's the idea here and I'm not getting it 😁

I probably don't exactly understand the risks here for our own codebase or how you stumbled across this issue, so it would just be completely subjective from my side, maybe@max-wittig or@lmilbaum would be better to take a look here :P

@nejch Not sure I am following the discussion. Will take a deeper look at it in few hours.

I've done some reading to figure out what the underscore means in the file name and its impact on time those lines of code are executed. Would you mind helping me figure this out?

I think it's to indicate a private module, and it is imported here to ensure the code is evaluated before any other logging code. But I'll let John explain this :)

Just a thought, but how about just doing a simplelogging.getLogger("gitlab") in our code as that would make all the string manipulation and tests unnecessary.

If the package name changes, users would have much bigger problems than this, as they would not be able toimport gitlab.

IIRC this PR was started before we centralized the licensing into one file only, so maybe can be removed? Or if you feel strongly about license per file maybe just an SPDX header to keep it concise and machine-readable.

I'll get rid of that 👍

To me it just shifts the issue to imports if something happens to be imported e.g.import _clients would go before this. So I thought there might be a simple way for this, as I don't really see it done in popular python libraries. I know some useget_logger() helpers to lazily load loggers, maybe that's the idea here and I'm not getting it 😁

I probably don't exactly understand the risks here for our own codebase or how you stumbled across this issue, so it would just be completely subjective from my side, maybe@max-wittig or@lmilbaum would be better to take a look here :P