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.

Sign up for GitHub

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

Jump to bottom

Write logs to cbi.log by default #112

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Write logs to cbi.log by default #112

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d10f9a
Simplify usage of logging in main script
Pennycook Sep 11, 2024
296ea50
Adjust logging configuration
Pennycook Sep 11, 2024
6c055b3
Use abspath to print cbi.log location
Pennycook Sep 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions codebasin/__main__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from codebasin import CodeBase, config, finder, report, util
from codebasin.walkers.platform_mapper import PlatformMapper

log = logging.getLogger("codebasin")
version = "1.2.0"


Expand Down Expand Up @@ -144,12 +145,25 @@ def main():

args = parser.parse_args()

stdout_log = logging.StreamHandler(sys.stdout)
stdout_log.setFormatter(logging.Formatter("[%(levelname)-8s] %(message)s"))
logging.getLogger("codebasin").addHandler(stdout_log)
logging.getLogger("codebasin").setLevel(
max(1, logging.WARNING - 10 * (args.verbose - args.quiet)),
)
# Configure logging such that:
# - All messages are written to a log file
# - Only errors are written to the terminal by default
# - Messages written to terminal are based on -q and -v flags
formatter = logging.Formatter("[%(levelname)-8s] %(message)s")
log.setLevel(logging.DEBUG)

file_handler = logging.FileHandler("cbi.log", mode="w")
file_handler.setLevel(logging.INFO)
file_handler.setFormatter(formatter)
log.addHandler(file_handler)
log_path = os.path.abspath("cbi.log")
print(f"Log file created at {log_path}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you not just log this after you've set up the stdout handler?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. This is a tricky one, I think.

Written this way, the message is part of CBI's output. It's always printed to the terminal, regardless of how many -q or -v flags the user passed. It's (hopefully) very difficult for a user to miss this message, but it won't be logged.

If we wrote this to the log using log.info, it wouldn't show up to stdout by default. A user would have to run with -vv to see it, and that would also turn on all the warnings and other information messages, too.

One possible, but slightly hacky, solution would be to write it like this:

log.addHandler(stdout_handler)
log.info(f"Log file created at {log_path}")
stdout_handler.setLevel(log_level)

Logging before we call setLevel means the message displays unconditionally, but also gets written to the log. There's probably a smarter way to set up different logging rules for __main__.py, but that might be overkill if we only care about this one message.

Is that too much of a hack, or do you think it's okay?


log_level = max(1, logging.ERROR - 10 * (args.verbose - args.quiet))
stdout_handler = logging.StreamHandler(sys.stdout)
stdout_handler.setLevel(log_level)
stdout_handler.setFormatter(formatter)
log.addHandler(stdout_handler)

# If no specific report was specified, generate all reports.
# Handled here to prevent "all" always being in the list.
Expand Down Expand Up @@ -243,5 +257,5 @@ def report_enabled(name):
sys.argv[0] = "codebasin"
main()
except Exception as e:
logging.getLogger("codebasin").error(str(e))
log.error(str(e))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this purposefully done to not include the traceback? Otherwise you can do log.exception(e)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly. When codebasin is run as a user-facing application, we want to catch all the exceptions and not reveal any traceback information.

sys.exit(1)
Loading