Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cli): watch paths for auto uploading daemon #14923

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

eligao
Copy link
Contributor

@eligao eligao commented Dec 25, 2024

Adds a --watch flag to continue watching for changes after initial upload.

  • Example: immich upload ./assets/ --watch
  • Initial upload behaves just like without --watch, as it batch uploads all current items in one go.
  • The script then starts watching for file change/addition, with a simple batching logic for performance:
    • A batch will be fired on 100 file changes
    • Or 10 seconds after the last change
    • The numbers above are hardcoded for now, they are fine and probably not necessary to make it configurable?
  • Another --no-progress flag to hide progress bars to make logs cleaner when running as a daemon
    • This is implied by the --watch flag
  • Docker image: https://github.com/eligao/immich/pkgs/container/immich-cli

@github-actions github-actions bot added the cli Tasks related to the Immich CLI label Dec 25, 2024
@eligao eligao marked this pull request as draft December 25, 2024 10:35
@eligao eligao marked this pull request as ready for review December 26, 2024 19:18
@eligao eligao force-pushed the feat/cli-watch-changes branch from 78824da to 32ba427 Compare December 26, 2024 19:24
cli/src/commands/asset.spec.ts Outdated Show resolved Hide resolved
cli/src/commands/asset.ts Outdated Show resolved Hide resolved
cli/src/commands/asset.ts Show resolved Hide resolved
cli/src/commands/asset.ts Show resolved Hide resolved
cli/src/commands/asset.ts Outdated Show resolved Hide resolved
@eligao eligao force-pushed the feat/cli-watch-changes branch from c267202 to 3e74c98 Compare December 30, 2024 07:42
@eligao eligao requested a review from jrasm91 January 14, 2025 14:31
batchSize,
debounceTimeMs,
onBatch: async (paths: string[]) => {
const uniquePaths = [...new Set(paths)];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would these ever not be unique?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chokidar.watchFs() can emit multiple events when a file is created/written/closed.
It is worked around with awaitWriteFinish but it's safer to double check here for duplication.

cli/src/commands/asset.ts Outdated Show resolved Hide resolved
if (!extensions.has(ext ?? '')) {
return;
}
console.log(`Change detected: ${path}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this interact with the progress bar?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, let me disable this log to prevent conflict when progress bar is enabled

@@ -69,6 +69,13 @@ program
.default(4),
)
.addOption(new Option('--delete', 'Delete local assets after upload').env('IMMICH_DELETE_ASSETS'))
.addOption(new Option('--no-progress', 'Hide progress bars').env('IMMICH_PROGRESS_BAR').default(true))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this hiding progress by default? The option and the env name are confusing me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this option starting with --no- is suitable for the option defaulting to true. The final parsed progress is not negated.

when running as a daemon, rendering a progress bar might flood the logs with duplicated progress bar re-renders, thus I imply progress to be false when watch is true below. progress is still true by default.

see: https://www.npmjs.com/package/commander#other-option-types-negatable-boolean-and-booleanvalue

cli/src/utils.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:enhancement cli Tasks related to the Immich CLI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants