Refactor CLI to use dependency injection

[mescanne]

No description provided.

[gemini-code-assist]

Summary of Changes

Hello @mescanne, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

I've refactored the command-line interface (CLI) to adopt a more robust dependency injection pattern. This change significantly improves the modularity and testability of the CLI by centralizing the creation and provision of core components like the database backend and the Ente client. Instead of manually passing context objects or retrieving values from a shared context dictionary, components are now automatically injected where needed, making the code cleaner and easier to maintain.

Highlights

• Backend Dependency Injection: I've introduced a new get_backend function that leverages Typer's dependency injection system to provide the appropriate database backend (either in-memory or SQLite) to the application. This centralizes backend creation and makes it easily configurable via command-line options.
• Client Dependency Injection: The get_client function has been refactored to receive its dependencies, such as the backend instance and various API URLs, through Typer's dependency injection. This eliminates the need to pull these values from the typer.Context.obj dictionary, leading to cleaner and more explicit dependency management.
• Streamlined CLI Commands: All CLI commands, including link, info, refresh, export, upload, download_missing, and download, have been updated to directly receive the EnteClient instance as an injected dependency. This simplifies their signatures and internal logic, as they no longer need to explicitly call get_client(ctxt).
• Refactored Main Application Callback: The app_main callback function has been streamlined by removing parameters that are now handled by the new dependency injection setup (e.g., backend, api_url, database). Its role is now more focused on general application context like the synchronization directory and maximum versions.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request successfully refactors the CLI to use Typer's dependency injection, which greatly improves the structure and testability of the commands. The introduction of get_backend and get_client as dependencies cleans up the command functions significantly.

My review includes a few suggestions for further improvement:

• A critical bug in the upload command that would cause a crash.
• An opportunity to complete the dependency injection refactoring for commands that still use typer.Context.
• Removal of an unused parameter in app_main.

Addressing these points will make the CLI more robust and consistent in its design.

[gemini-code-assist]

This code will raise an AttributeError because SQLiteBackend does not have a db_path attribute. This will cause the upload command to crash when using an SQLite backend.

Accessing client.backend.db_path is also problematic because it relies on an implementation detail of the backend.

A safer way to get this information for logging, without modifying other files, is to get it from the engine attribute which is part of SQLiteBackend.

Suggested change

	db_path = None
	if isinstance(client.backend, SQLiteBackend):
	db_path = client.backend.db_path
	log.info("sync_dir: %s database: %s", ctxt.obj["sync_dir"], db_path)
	db_path = None
	if isinstance(client.backend, SQLiteBackend):
	db_path = str(client.backend.engine.url).removeprefix("sqlite:///")
	log.info("sync_dir: %s database: %s", ctxt.obj["sync_dir"], db_path)

[gemini-code-assist]

The refresh and upload commands still depend on typer.Context to get sync_dir. While the comment in app_main notes that the context is for app-level state not suitable for a dependency, sync_dir seems like a good candidate for being provided via dependency injection. This would make the command's dependencies more explicit, remove the need for the context object in these command functions, and align better with the overall goal of this refactoring.

You can create a dependency for sync_dir and use it where needed. This would centralize the logic for determining and validating the sync_dir.

Here's an example of how you could define a get_sync_dir dependency (you'd place this with your other dependency functions):

def get_sync_dir(
    sync_dir: Annotated[
        Path | None, typer.Option(help="Local synchronization directory")
    ] = None,
) -> Path:
    """Get and validate the sync directory."""
    if sync_dir is None:
        sync_dir = Path.home()

    if not sync_dir.exists():
        log.error("Sync directory %s does not exist.", sync_dir)
        raise typer.Exit(1)
    return sync_dir

Then you can inject it into refresh and remove the ctxt parameter. A similar change could be applied to upload.

Suggested change

	def refresh(
	ctxt: typer.Context,
	force_refresh: Annotated[bool, typer.Option()] = False, # noqa: FBT002
	client: Annotated[EnteClient, typer.Depends(get_client)],
	force_refresh: Annotated[bool, typer.Option(show_default=False)] = False,
	email: Annotated[str | None, typer.Option()] = None,
	workers: Annotated[int | None, typer.Option()] = None,
	) -> None:
	"""Refresh both remote and local data."""
	client = get_client(ctxt)
	client.remote_refresh(email=email, force_refresh=force_refresh)
	client.local_refresh(ctxt.obj["sync_dir"], force_refresh=force_refresh, workers=workers)
	def refresh(
	client: Annotated[EnteClient, typer.Depends(get_client)],
	sync_dir: Annotated[Path, typer.Depends(get_sync_dir)],
	force_refresh: Annotated[bool, typer.Option(show_default=False)] = False,
	email: Annotated[str | None, typer.Option()] = None,
	workers: Annotated[int | None, typer.Option()] = None,
	) -> None:
	"""Refresh both remote and local data."""
	client.remote_refresh(email=email, force_refresh=force_refresh)
	client.local_refresh(sync_dir, force_refresh=force_refresh, workers=workers)

[gemini-code-assist]

The max_vers option is defined and stored in the context object, but it doesn't appear to be used anywhere in the application. It seems to be dead code and could be removed to simplify the CLI. If it's intended for future use, a TODO comment would be helpful.