cmu-delphi / delphi-epidata

An open API for epidemiological data.
https://cmu-delphi.github.io/delphi-epidata/
MIT License
100 stars 68 forks source link

py client: drop delphi_utils requirement, log to stderr instead #1486

Closed melange396 closed 2 months ago

melange396 commented 3 months ago

This drops the requirement for the bloated delphi_utils package in the python client, and replaces the delphi_utils.logger with printing to STDERR instead.

Addresses #1467

I havent tested this yet, and i imagine it is going to fail CI.

sonarcloud[bot] commented 3 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

dshemetov commented 3 months ago

Seems like a simple enough workaround 👍 .


I was curious to learn a little about stderr and structlog. A few basic notes I found helpful, for future reference:

melange396 commented 3 months ago

@dshemetov youre right, the FileHandler stuff is basically unreachable. One of the googlebros borked it, but it wasnt much better before that anyway... I guess we are lucky that we never needed to directly write to a log file with this!

dshemetov commented 3 months ago

Ah well this actually answers a question that came up in a conversation with @aysim319: I was wondering whether any of these get_structured_logger calls actually depended on that filename argument in any important way and looks like the answer is no. Since it's been like this for 3 years, we must not rely on those file logs. On the plus side, that suggests that we can remove that argument, so the logger doesn't depend on the params file, so it can be pulled out of the run function, set as a global in each module, and then we can stop passing the logger around from function to function.

Update: alright, so the good news is that the FileHandler branch is actually reached (which allows indicator-specific log files). The way this works out is that as long as the name variable is non-empty in logging.getLogger(), the returned logger is a child logger which don't have handlers by default:

import logging

logging.basicConfig(
    format="%(message)s",
    level=logging.DEBUG,
    handlers=[logging.StreamHandler()] # This is the default handler
)
# Root logger gets the handler
root_logger = logging.getLogger()
assert root_logger.handlers
# Named loggers have no handlers by default
named_logger = logging.getLogger("named")
assert not named_logger.handlers