Improve usability

[dragoangel]

Summary

Changes for end user:

• cli arguments names changed
• config migrated to new format
• writing of logs to directory disabled by default for new users (who not have old config)

Fixes #82

cvdupdate/cvdupdate.py

• All config keys normalized to snake_case (logs_directory, dbs_directory, etc.)
• __init__ accepts all 11 config options as overridable parameters using sentinel defaults ("" / 0 / None = don't override)
• _read_config applies sentinel-based overrides and persists only changed values
• _init_logging gates FileHandler creation behind logs_enabled
• config_add_db supports override=False parameter for --override flag
• max_retries clamped to range 1–5 on load; out-of-range values (including persisted ones) are reset to default 3
• Removed dead code: empty update() stub, config_show() method, import subprocess

cvdupdate/__main__.py

• Added AliasedGroup(click.Group) — aliases resolved transparently and displayed inline in help: status (s), update (u), list (ls), remove (rm), config (cf), clean (cl)
• config set exposes all 12 config options; falls back to --help when called with no options
• config show outputs only the config JSON (removed state section)
• status replaces list logic when called without argument, support --json argument to display status file, when called with name of database - only 1 db status will be shown (also support --json argument
• list now only provide names of dbs to simplify parsing
• added --override flag for add so user can adjust remote URL for existing database
• serve defaults to port 0 (OS picks a free port); actual port logged after bind
• Removed dead code: from pathlib import Path, Back from colorama, module_logger

scripts/docker-entrypoint.sh

• Updated CLI flags to match new names: --logs-directory, --dbs-directory
• Config args built as a CONFIG_ARGS array shared across root/non-root branches
• ENABLE_LOGS — enables file logging when set to any value except false
• LOGS_TO_KEEP — sets log rotation count; validated as a positive integer (^[1-9][0-9]*$), warns and skips on invalid
• EXTRA_DATABASES — comma-separated <db_name>:<db_url> entries, calls cvdupdate add --override for each; validates both fields present
• REMOVE_DATABASES — comma-separated db names, calls cvdupdate remove for each
• Both loops use index-based iteration to avoid word-splitting issues

compose/httpd.conf + compose.yaml

• New minimal Apache config: DocumentRoot /cvdupdate/database with Options Indexes for directory listing, previously this listing of files in directory was not possible
• compose.yaml mounts ./compose/httpd.conf into the existing apache service

[dragoangel]

Hi @val-ms, sorry for pinging, but there no auto reviewers and nobody checking it for a while, can you help with this? Reasons why I created this PR described in issue created by me - liked to this PR.

[val-ms]

Hi @dragoangel I've just been a bit behind on things. I will try to catch up and review it soon.

[val-ms]

Thanks for putting this together. I like the direction of the PR: fuller config set coverage, JSON output, add --override, less noisy default logging for container/systemd usage, Docker configurability, and the compose Apache serving fix all address real usability pain from #82.

I am requesting changes because several of the improvements currently land as breaking changes without a deprecation path:

1. show was removed and replaced by status. status is the better command name and the new command is more capable, but existing cvd show <db> / cvdupdate show <db> users will break. Please keep show as a deprecated compatibility alias or wrapper for at least one release, ideally with a warning pointing users to status.

2. config set renames/removes existing flags such as --logdir, --dbdir, and --nameserver, and the DNS environment variable appears to move from CVDUPDATE_NAMESERVER to CVDUPDATE_NAMESERVERS. The new names are clearer, but please preserve the old flag/env aliases and update README/CHANGES to document the transition.

3. serve changes the default port from 8000 to 0. Supporting explicit 0 for OS-selected random port selection is useful, but the default should remain 8000 for compatibility and to match existing docs/user expectations. Please restore the default to 8000, keep support for passing 0, and document in the serve help text that 0 requests a random available port.

4. The new default state_file is in the database directory as state.json, and that directory is served by cvd serve and by the compose Apache service. That risks exposing state metadata. Please keep state out of served content by default, or make it hidden/excluded robustly for both the built-in server and Apache config.

Please also add CLI-level tests for the compatibility aliases and the new status/JSON behavior. The included tests cover migration/logging and pass locally for me in an isolated HOME, but they do not exercise the Click command surface, Docker entrypoint behavior, or the serving behavior affected by this PR.

[dragoangel]

Hi @val-ms thanks, clear, will fix ☺️

[dragoangel]

@val-ms please take a look, I also updated Readme in my PR and added changes, if readme/changes unnecessary I can revert part.

[val-ms]

Thanks for the update. The original compatibility concerns are mostly addressed now: show is retained as a deprecated alias, the old config flags and singular nameserver env var are accepted, serve defaults back to 8000 while still accepting 0, the default state file is no longer in the default served database directory, dot-prefixed paths are hidden, and the current head merges cleanly with PR #87.

I am still requesting changes for a few remaining issues:

1. Custom --config paths no longer keep state beside that config by default. On main, when a user passes something like --config /tmp/foo/config.json and does not explicitly set a state file, the state file defaults to /tmp/foo/state.json. In this PR it defaults to $HOME/.cvdupdate/state.json because default_config["state_file"] is already expanded from Path.home(). That breaks isolated alternate config usage. Please keep the normal default state file outside the database directory, but preserve the existing behavior that custom config files get a colocated default state file.

2. The deprecated --logdir compatibility flag does not preserve the old behavior. cvd config set --logdir /tmp/logs maps the directory to logs_directory, but leaves logs_enabled as false, so file logging is disabled. Since the old --logdir command implied file logging, the alias should enable logs unless the user explicitly disables them.

3. The requested CLI tests still appear to be missing. The commit message mentions tests for aliases, status/JSON, deprecated flags, and serve hiding, but the PR currently only has library-level tests in tests/test_cvdupdate.py; I do not see any CliRunner or command-surface tests. Please add tests for at least show as a deprecated alias, status --json, legacy --logdir/--dbdir/--nameserver, and serve hiding dot-prefixed paths including percent-encoded dot paths.

Verification from my re-review:

• pytest tests: 5 passed
• bash -n scripts/docker-entrypoint.sh: passed
• Docker image build: passed
• Manual CLI probes confirmed show and deprecated config flags mostly work, except for the --logdir behavior above.

Compatibility note: this PR now merges cleanly with current #87 and retains the managed cron block. It still conflicts with #81 in CHANGES.md, cvdupdate/__main__.py, and cvdupdate/cvdupdate.py, so whichever of #81/#88 lands second will need a rebase/reconciliation.

[dragoangel]

Done

[val-ms]

Thanks for all the hard work on this, especially for sticking with our somewhat pedantic compatibility and test coverage requests. The previous blockers are mostly addressed now: custom config paths colocate state again, deprecated --logdir enables file logging unless explicitly disabled, the CLI tests are present, and the dotfile serving coverage is in place.

I am still requesting changes for two remaining CLI behavior issues from this re-review:

1. status --json does not report the same database set as text status.

   In cvdupdate/__main__.py, the text path calls m.db_list(), which goes through _index_local_databases() and includes locally present database files that are not already in state. The JSON path prints raw m.state, so those local files are omitted. I reproduced this by placing a local.hdb in the configured database directory: cvd status --config ... showed local.hdb, while cvd status --config ... --json did not. Since --json is the machine-readable form of status, it should use the same indexed database view as the text path.

2. The deprecated hidden show command is still displayed in top-level help.

   show is registered with hidden=True, but AliasedGroup.format_commands() builds the command list manually and does not skip hidden commands. As a result, cvd --help still lists show as a normal command. Please preserve Click hidden-command behavior while adding the alias display, for example by filtering out cmd.hidden.

Verification from this pass:

• HOME=/private/tmp/cvdupdate-pr88-home PYTHONPATH=.venv/lib/python3.9/site-packages /opt/homebrew/bin/pytest tests: 23 passed
• bash -n scripts/docker-entrypoint.sh: passed
• Merging the PR head into current origin/main in a temporary worktree was clean and preserved PR #87 managed cron block

[dragoangel]

Fixed, thanks for heads up.

[val-ms]

It looks good to me! Thank you for your patience with me, and for the contribution.