ENH: new cli command init

untzag commented 1 year ago

Description

Add a new CLI command happi init. This command creates a new config file in the default place pointing to an empty json database in the default place.

If an existing config file is found happi init will refuse to overwrite it unless you use the (hopefully scary) incantation happi init --overwrite.

Command tells you where the config and database files are being put.

Motivation and Context

Intent is to make it easy for users to "bootstrap" brand new happi installs.

How Has This Been Tested?

CLI, works on my machine.

Where Has This Been Documented?

CLI is auto-documented.

Pre-merge checklist

[x] Code works interactively
[x] Code contains descriptive docstrings, including context and API
[x] New/changed functions and methods are covered in the test suite where possible
[x] Test suite passes locally
[x] Test suite passes on GitHub Actions
[x] Ran docs/pre-release-notes.sh and created a pre-release documentation page
[x] Pre-release docs include context, functional descriptions, and contributors as appropriate

untzag commented 1 year ago

I can imagine this idea might be controversial to y'all, so feel free to reject. Also feel free to request changes, we could remove the option to overwrite or we could change the default locations if there is a different preference among maintainers.

tangkong commented 1 year ago

My first impression of this is that it's a nice addition. It does expose the config to tampering, but we should probably just restrict access to that at the file level anyway.

Testing this out revealed that when I reworked the happi cli I assumed the happi config exists. So running this command without a happi config will throw an OSError from find_config. This also affects the edit-config case, but I missed that in my review.

It's not immediately clear to me what the right change here is. Things that came to mind:

let the happi_cli group pass None if the client is not found, and make all the subcommands (everything except init?) that rely on the client no-ops
Make a separate group for subcommands that don't involve an instantiated client (edit-config, init). Something like happi-admin? Would require setting up another entrypoint/script in the pyproject.toml

untzag commented 1 year ago

I missed that too @tangkong, not sure how I missed it...

I vote for allowing None, for what it's worth. Feel free to push any edits you care to make right into this branch.

klauer commented 1 year ago

Would a set of happi config commands make sense? happi config init (this PR) happi config edit (previous PR) happi config show (show the config settings; insert other ideas here)

As for the None thing - I think individual CLI commands should just be calling some helper function as needed and happi_cli shouldn't be making the client instance straightaway now that we're thinking about these config-less helpers.

client = get_cli_client(ctx) - some cached singleton used only in the CLI, using the top-level happi_cli context settings, for example. Alternatively, maybe the context could get a new method (client = ctx.get_client()) - admittedly I'm not really a fan of this pattern though.

untzag commented 1 year ago

I like your idea @klauer. I could implement it tomorrow if everyone agrees.

tangkong commented 1 year ago

I like that proposal. Adding a sub-group is easy enough

@click.group()
def happi_cli(...):
    ...

@click.group()
def config(...):
    ...

@config.command(...)
def init(...):
    ...

if __name__ == "__main__":
    happi_cli.add_command(config)
    happi_cli()

I spent some time trying to figure out how to hide the other client-related subcommands if the provided path is not found, but couldn't quickly find a nice method. There's @click.command(hidden=True) kwarg, but that's not evaluated dynamically (read: after happi_cli takes in a config path).

I'll let @untzag take a stab, but I'm happy to chat about this if I can be of help.

pcdshub / happi