Add console module

[JimMadge]

Closes #1945

[jemrobinson]

I agree that logging and writing to console are distinct actions, but I was wondering whether there's a sufficiently broad module name that could cover them both. They're more similar to one another than e.g. logging is to deploying Pulumi infrastructure. What do you think @JimMadge?

[JimMadge]

I don't like the idea of both in a module at the same level, like output.logger.info() and output.pretty_print. Having nested modules, like output.logging.logger and output.console.pretty_print, seems over complicated.

[jemrobinson]

I guess I was imagining something like:

from data_safe_haven.output import get_logger, console

logger = get_logger()
logger.info("message")
console.print("message")

to replace

from data_safe_haven.logging import get_logger
from data_safe_haven.console import console

logger = get_logger()
logger.info("message")
console.print("message")

Not a big difference, but might make the code a bit easier to understand for new developers.

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
data_safe_haven/administration/users
user_handler.py
data_safe_haven/commands
cli.py
config.py
context.py
pulumi.py
data_safe_haven/console
__init__.py
format.py
pretty.py
data_safe_haven/external/api
azure_cli.py					75
graph_api.py					1112
data_safe_haven/utility
__init__.py
Project Total

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

[JimMadge]

I also should add tests for the parts that aren't covered given that I have had a look at them here.

[JimMadge]

@jemrobinson I'm still not keen on collecting logging and printing in one module.

Would something like

from data_safe_haven.logging import get_logger
from data_safe_haven.console import console, prompts

logger = get_logger()
logger.info("hello")
console.print("hello")  # console is a rich.console.Console
prompts.confirm("yes or no?")

be better?

[jemrobinson]

I think that looks OK (did you mean to import prompts? I think you wouldn't need to in this case). Essentially, I think it would be helpful to have console.method calls which look like logger.method calls so you can easily see "ah, this is being logged but this is going straight to the console".

N.B. I don't mean that we need console.info etc., console.print is fine.

[JimMadge]

Oh, I made a mistake, I've updated the code block.

[jemrobinson]

OK, what do you think of:

from data_safe_haven.logging import get_logger
from data_safe_haven.console import console

logger = get_logger()
logger.info("hello")
console.print("hello")
console.confirm("yes or no?")

instead?

[JimMadge]

Yes we can do that. The only thing that is a little annoying about that is we are making a wrapper for rich.Console.print instead of just exposing a rich.Console. Not a big deal though and it does make the API much cleaner.

[jemrobinson]

I agree, it's a bit annoying - maybe we can use this chance to provide an adaptor for the console.print arguments that fits our needs better (e.g. we probably don't use all the arguments at the moment)?

[jemrobinson]

LGTM. Does it work? :)

[JimMadge]

Yes, as far as I can tell.

I'm still not sure what is happening in #1954. I can't reproduce it with dsh config upload ....