Logging

[JimMadge]

✅ Checklist

• You have given your pull request a meaningful title (e.g. Enable foobar integration rather than 515 foobar).
• You are targeting the appropriate branch. If you're not certain which one this is, it should be develop.
• Your branch is up-to-date with the target branch (it probably was when you started, but it may have changed since then).

🚦 Depends on

⤴️ Summary

• Logging changes
  • Simplify handler/formatter construction
  • Remove log level, file, line number from console handler
• Move unrelated methods from logging class
• Remove top level try/except
• More Pythonic use of logging module (makes configuration neater, particularly in unit tests)

The approach here is

• ConsoleHandler: Logging to console, information for the user, INFO and above
• PlainFileHandler: Logging to a file with rich markup removed, information for auditing and debugging, all levels

with logging levels

• rich.print: When output is only of interest to the user. Used for commands which send their output to stdout.
• logger.info: Used for notifications of actions taken and completed successfully. stdout which is also useful to log.
• logger.warning: Used for unexpected events or potential problems that won't cause the program to stop. User should take notice even if the program continues.
• logger.error: Used for unexpected events or problems that may cause the program to stop.
• logger.fatal: Used for problems that will stop the program, immediately before an exit call.

🌂 Related issues

Closes #1891
Closes #1657

🔬 Tests

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
data_safe_haven
__init__.py
directories.py
data_safe_haven/administration/users
entra_users.py					29
user_handler.py					29, 129, 155, 161
data_safe_haven/commands
cli.py					57, 60, 100
config.py
context.py
pulumi.py
sre.py					32
users.py					128, 230
data_safe_haven/config
config.py
data_safe_haven/context
context_settings.py
data_safe_haven/exceptions
__init__.py
data_safe_haven/external/api
azure_api.py					753
azure_cli.py					75
graph_api.py					87, 1086, 1112-1119
data_safe_haven/external/interface
azure_postgresql_database.py					54, 151, 159
data_safe_haven/infrastructure
project_manager.py					176, 251-254, 262, 272, 306
data_safe_haven/infrastructure/components/dynamic
entra_application.py					43, 69, 125
ssl_certificate.py					51, 81, 156
data_safe_haven/logging
__init__.py
logger.py
data_safe_haven/provisioning
sre_provisioning_manager.py					32, 117
data_safe_haven/utility
console.py					17-25
prompts.py
Project Total

The report is truncated to 25 files out of 31. To see the full report, please visit the workflow summary page.

This report was generated by python-coverage-comment-action

[jemrobinson]

Something to consider (no problem if you decide not to) - should we log all runs to $appdirs / $date-$user-something.log and potentially also upload these to Azure for future reference?

[jemrobinson]

Overall, LGTM. Can you post some examples of what the output looks like before and after for places where it's changed?

[JimMadge]

Merged through docs link check failing as that wasn't modified here. Does look like a genuine failure though.

[JimMadge]

Screenshots,

• New with --verbose --show-level
• New (default)
• Old

[JimMadge]

And the log file,

2024-06-20 11:13:47,048 - DEBUG - Reading project settings from '/Users/jmadge/Library/Application Support/data_safe_haven/contexts.yaml'.
2024-06-20 11:13:47,067 - INFO - Switched context to 'elements'.
2024-06-20 11:13:47,073 - DEBUG - Saved context settings to '/Users/jmadge/Library/Application Support/data_safe_haven/contexts.yaml'.
2024-06-20 11:14:06,576 - DEBUG - Reading project settings from '/Users/jmadge/Library/Application Support/data_safe_haven/contexts.yaml'.
2024-06-20 11:14:06,586 - INFO - Switched context to 'elements'.
2024-06-20 11:14:06,595 - DEBUG - Saved context settings to '/Users/jmadge/Library/Application Support/data_safe_haven/contexts.yaml'.

[jemrobinson]

Looks great. I like the idea of keeping timestamps in the log file but not in the console output (by default). Do you think there's any benefit in adding an option to include timestamps in the console output?

[JimMadge]

Looks great. I like the idea of keeping timestamps in the log file but not in the console output (by default). Do you think there's any benefit in adding an option to include timestamps in the console output?

That did occur to me when I posted the pictures. Could definitely do that too, should be an easy addition.