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

Added option to download X oldest items, not only X recent ones (building upon Santa77's #620) #1022

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

lcorbasson
Copy link

Added option to download X oldest items, not only X recent ones (see PR #620 by Santa77).

Synced with the last version in master.

@AndreyNikiforov
Copy link
Collaborator

What is the use case for this functionality? I can see how "--recent" helps with optimizing incremental updates (and with testing), but I fail to see usage of the "--oldest" flag.

@lcorbasson
Copy link
Author

lcorbasson commented Dec 19, 2024

This is useful to delete the oldest pictures in the iCloud library while keeping the most recent ones, e.g.:
icloudpd -d "$ICLOUD_PICTURES_LOCAL_DIRECTORY" -u "$ICLOUD_USERNAME" --delete-after-download --oldest 20000

@roju
Copy link

roju commented Dec 19, 2024

My iCloud storage is full because there are too many old photos. I want to download some of these old photos, and then delete after download. I also want to keep a certain number of recent photos in my iCloud account at all times.

@AndreyNikiforov
Copy link
Collaborator

This is useful to delete the oldest pictures in the iCloud library while keeping the most recent ones, e.g.: icloudpd -d "$ICLOUD_PICTURES_LOCAL_DIRECTORY" -u "$ICLOUD_USERNAME" --delete-after-download --oldest 20000

That is quite useful! I haven't though about such use case.

@@ -582,6 +587,7 @@ def main(
size: Sequence[AssetVersionSize],
live_photo_size: LivePhotoVersionSize,
recent: Optional[int],
oldest: Optional[int],
Copy link
Collaborator

Choose a reason for hiding this comment

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

What params cannot be used with --oldest? I think at least --recent. Need to check for invalid combination of params.

@@ -935,6 +943,7 @@ def download_photo_(counter: Counter, photo: PhotoAsset) -> bool:
if file_exists:
counter.increment()
logger.debug("%s already exists", truncate_middle(download_path, 96))
success = True
Copy link
Collaborator

Choose a reason for hiding this comment

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

That logic change seems to be related to cases outside of oldest as well. Will is break anything?

@@ -1289,6 +1299,13 @@ def core(
photos_count = recent
photos_enumerator = itertools.islice(photos_enumerator, recent)

# Optional: Only download the x oldest photos.
if oldest is not None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add tests for new functionality. That will ensure this feature is not broken with other code changes.

photos_count = oldest
total_photos = len(photos_enumerator)
start = total_photos - oldest
photos_enumerator = itertools.islice(photos_enumerator, start, total_photos)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am curious how that works on large collections in iCloud.com. I think we are processing images as a stream and any filtering is done on a fly instead of memory.

@AndreyNikiforov
Copy link
Collaborator

This is useful to delete the oldest pictures in the iCloud library while keeping the most recent ones, e.g.: icloudpd -d "$ICLOUD_PICTURES_LOCAL_DIRECTORY" -u "$ICLOUD_USERNAME" --delete-after-download --oldest 20000

@lcorbasson There may be another option to solve your use case: tweak delete-after-download option. E.g. "Delete after downloading if older than 30days from now except favorites". That may be useful to broader audience who keep iCloud.com storage to min. It may worth discussing all edge cases and analyze reported issues before coding though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants