Skip to content
This repository has been archived by the owner on Jul 25, 2024. It is now read-only.

Stop logging with eagerly-formatted strings #157

Merged
merged 4 commits into from
Apr 25, 2024
Merged

Conversation

samuelhwilliams
Copy link
Contributor

https://dluhcdigital.atlassian.net/browse/SMD-812

Change description

Stop eagerly formatting log messages, which ends up causing some log messages to be double-formatted. When the initial format is injecting a dictionary into the string, the second format (by fsd_utils.logging) can error because the dict looks like a format injection point, but is incorrectly structured.

I've also switched black+flake8+isort out for ruff, which is a fast modern tool that combines a bunch of formatting and linting rules. One of these will raise linting issues for log call that have eagerly-formatted log lines.

  • Unit tests and other appropriate tests added or updated
  • README and other documentation has been updated / added (if needed)
  • Commit messages are meaningful and follow good commit message guidelines (e.g. "FS-XXXX: Add margin to nav items preventing overlapping of logo")

@samuelhwilliams samuelhwilliams marked this pull request as ready for review April 24, 2024 15:59
pyproject.toml Outdated Show resolved Hide resolved
This is a single tool that replaces three, and is generally seeing a lot
of development and improves recently. It's also generally faster than
all of those tools.

We also enable some extra 'flake8-equivalent' linting rules,
specifically here one that blocks eagerly-formatted strings in log
lines.
Don't use f-strings in logging calls - leave it to be handled by
fsd_utils.
Copy link
Contributor

@gidsg gidsg left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@samuelhwilliams samuelhwilliams merged commit 8cd364f into main Apr 25, 2024
2 checks passed
@samuelhwilliams samuelhwilliams deleted the bau/fix-logging branch April 25, 2024 12:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants