Remove pyicloud_ipd fork

[AndreyNikiforov]

@ndbroadbent what is in pyicloud_ipd fork that cannot be merged into upstream? Using upstream will hopefully reduce support scope.

[ndbroadbent]

Yes I totally agree, I think it's definitely time to do this! That would be great! There's a ton of minor changes and fixes, and I looked into this a few weeks ago before giving up. But I think it might not be too hard to get everything removed and updated.

These are all the changes in the current branch that is being used (pyicloud-ipd):

* 1d00ee5 - (HEAD -> pyicloud-ipd) Added build script (3 weeks ago) <Nathan Broadbent>
* 8f5bab0 - Fix release script (3 weeks ago) <Nathan Broadbent>
* 7899300 - Bump version (3 weeks ago) <Nathan Broadbent>
* 1f0d5c7 - (origin/pyicloud-ipd) Apply change from https://github.com/ndbroadbent/pyicloud/pull/4 (3 weeks ago) <Nathan Broadbent>
* 37e25bd - Bump version to 0.10.0 (1 year, 5 months ago) <Nathan Broadbent>
* a1a9f21 - Fix crash when album titles contain unicode characters (e.g. umlauts) (1 year, 5 months ago) <Nathan Broadbent>
* 5620b74 - Bump version to 0.9.9 (1 year, 10 months ago) <Nathan Broadbent>
* e3197df - Fix import in icloud cli tool (1 year, 10 months ago) <Nathan Broadbent>
* c16e67b - Bump version to 0.9.8 (1 year, 11 months ago) <Nathan Broadbent>
* 9e146ba - When filenameEnc is missing, build a filename from the first 12 characters of the sanitized fingerprint and the correct file extension (1 year, 11 months ago) <Nathan Broadbent>
* e8bc1cd - (tag: 0.9.7) Bump version to 0.9.7 (2 years ago) <Nathan Broadbent>
* 3ee4ea3 - Added an optional exception_handler to the PhotoAlbum photos iterator, so that we can catch and retry after authentication errors (2 years ago) <Nathan Broadbent>
* 5efd010 - Added install script to make development easier (2 years ago) <Nathan Broadbent>
* 28fd82a - Fix filenames for live photos (2 years, 2 months ago) <Nathan Broadbent>
* f741d8b - Added some build & release scripts (2 years, 2 months ago) <Nathan Broadbent>
* 8436efe - Bump version (2 years, 2 months ago) <Nathan Broadbent>
* 4223f4b - Added support for live photo versions (2 years, 2 months ago) <Nathan Broadbent>
* 17c94bc - use "not in" (2 years, 2 months ago) <Nathan Broadbent>
* a65dd96 - Fix bug (2 years, 2 months ago) <Nathan Broadbent>
* f4055fe - Bump version (2 years, 2 months ago) <Nathan Broadbent>
* 1b9e1ff - Return 'unknown' item_type if missing itemType key (2 years, 2 months ago) <Nathan Broadbent>
* 353b70b - Rename to pyicloud_ipd (2 years, 3 months ago) <Nathan Broadbent>
* 7d274e2 - Allow client_id to be overridden in __init__ (for replayable HTTP requests in pyvcr tests) (2 years, 3 months ago) <Nathan Broadbent>
* b65053d - Fix crash for unknown itemType. Fall back to checking the filename extension, then default to 'movie'. Fixes https://github.com/ndbroadbent/icloud_photos_downloader/issues/78 (2 years, 3 months ago) <Nathan Broadbent>
* 3225669 - Fix lint error (line too long) (2 years, 3 months ago) <Nathan Broadbent>

Here is my attempt at a rebase onto the latest version, but I think some things were broken: https://github.com/ndbroadbent/pyicloud/commits/pyicloud-ipd-rebase

[AndreyNikiforov]

@ndbroadbent as I understand, the following steps are needed:

1. merge pyicloud_ipd changes from fork to upstream repo
2. update icloudpd to use upstream version

If my understanding is correct, will you be able to complete first or both of these tasks?

[ndbroadbent]

Yes that's correct! But unfortunately I don't know if I will have enough time to work on these soon, so I don't mind if anyone else would like to take a look at it, but otherwise I can add it to my list and see if I can find some time in the future

[ndbroadbent]

I should also explain - I actually haven't used this software at all for the last 2 years, so that's why I'm no longer interested in actively maintaining it. But it's great that other people are still finding it useful, so thanks a lot for all your help!

[ndbroadbent]

Also I wasn't expecting you guys to be so active and contributing so much work, so that's great! In that case the project has definitely outgrown my personal GitHub account (although it was cool to have it under my name), but I think I'll create a new "icloud_photos_downloader" organization and move it there

[ndbroadbent]

I just set up the icloud-photos-downloader organization (unfortunately GitHub doesn't allow underscores in organization names, otherwise I would have used icloud_photos_downloader), and I've transferred over the icloud_photos_downloader and pyicloud (fork) repos into this organization.

GitHub will always maintain a redirect for the old ndbroadbent/ URLs or git remotes, so this shouldn't cause any problems.

[boredazfcuk]

Also I wasn't expecting you guys to be so active and contributing so much work, so that's great!

I'd like to thank you two as well. About a year ago, this project seemed to be dead in the water, but the pair of you really have breathed new life into it. The number of merged PR for bug fixes and feature upgrades over the past few weeks has been awesome. Thanks again.

[menkej]

The base project is still active and we could benefit from that. I completely understand that you're not using this project anymore and don't want to spend more time into it, @ndbroadbent ! I'm currently just playing around with this to reset my brain from my house building nightmare, day job, etc. and like to use it to get more into python and git(hub) while working on something useful.

I had my first bad encounter with a rebase this week, so I know its... special... Do you have any advice/idea how we together could get rid of that fork? I'm still considering myself a git newbie, 20 years with ClearCase don't help... ;-) so I' really struggling here...

btw: what do you guys think about some more interactive communication like slack or something?

[AndreyNikiforov]

btw: what do you guys think about some more interactive communication like slack or something?

I was thinking about that multiple times and always reminded myself to go back to more async communication with "paper"-trail (==GitHub issues). ;-)

[menkej]

"paper"-trail (==GitHub issues). ;-)

I agree that this has benefits for discussion about feature requests/bug(-fixes) and stuff and we should continue like that. But I had the impulse regarding delivery/release process and also some "I'm a noob and don't know how to handle GitHub"-things that might be easier to agree on or clarify in just a short more interactive discussion... Although we might have a little time-zone challenge...

[ndbroadbent]

Hey, I just wanted to follow up about these two outstanding steps:

1. merge pyicloud_ipd changes from fork to upstream repo
2. update icloudpd to use upstream version

Unfortunately I'm extremely busy at the moment with lots of things going on, so I don't have time to look into this, but it's definitely still a great idea to go back to using the original pyicloud library

[menkej]

There are quite some changes in pyicloud_ipd... It would be easier if our testsuite would behave more deterministic (especially test_download_photos). I can't complete a run without errors locally (Mac, Python 3.8.6) - on GitHub they pass most of the time. No explanation...

FAILED tests/test_download_photos.py::DownloadPhotoTestCase::test_download_photos_and_set_exif - AssertionError: 'DEBUG    Setting EXIF timestamp for tes...
FAILED tests/test_download_photos.py::DownloadPhotoTestCase::test_download_photos_and_set_exif_exceptions - AssertionError: 'DEBUG    Setting EXIF timest...
FAILED tests/test_download_photos.py::DownloadPhotoTestCase::test_handle_session_error_during_photo_iteration - AssertionError: assert 1 == -1
FAILED tests/test_download_photos.py::DownloadPhotoTestCase::test_invalid_creation_year - AssertionError: 'INFO     Downloading tests/fixtures/Photos/5/0...

Are the tests working for you locally?

[AndreyNikiforov]

These four tests are not working on mac and windows due to date issues. Send #215 to skip them for now. The rest are running fine on both dev platforms.

Note that tests on mac are VERY slow, like minutes(!). Running docker buildx build . --target test runs the same tests in 10sec on the same mac. Suspect it is related to the fact we are creating huge files as part of the tests -- layered code and unit testing would help in this case.

[menkej]

Note that tests on mac are VERY slow, like minutes(!). Running docker buildx build . --target test runs the same tests in 10sec on the same mac. Suspect it is related to the fact we are creating huge files as part of the tests -- layered code and unit testing would help in this case.

I already noticed that... :-( Unfortunately I get the following:

$ docker buildx
docker: 'buildx' is not a docker command.
See 'docker --help'
$ docker --version
Docker version 19.03.13, build 4484c46d9d

I have Docker 2.5.0.0 installed. Seems like Docker and I won't become friends soon...

[menkej]

Ok, forget about it. I had to enable "experimental features" and then buildx appeared.

Thanks a lot for the hint for running the tests like that, @AndreyNikiforov! I was always wondering what you use that Dockerfile for... How on hell can they be so fast!?! Its the same code running on the same iron?!?

[menkej]

I looked into the differences between upstream pyicloud and pyicloud_ipd rebase branch. At first glance I don't see so many differences apart from naming of movies (which we could handle in icloudpd and session-handling (which should go into upstream I guess). I'd really like to get rid from that fork to benefit from the original project which is still (more or less) active. As I still consider myself as a git(hub) and python beginner I'd need a little sparring on this. Could we possibly meet for a short interactive (Webex/zoom/whatever) session @AndreyNikiforov and/or @ndbroadbent to discuss the steps that need to be done? I'd sponsor the (virtual) beer. ;-)

[AndreyNikiforov]

@menkej I am not familiar with pyicloud codebase at all. I think the first step is to understand what these changes do, then check if we need them, then identify gap with upstream (may be some of that functionality is already implemented), then submit changes upstream or reuse upstream version with local customizations.

I am not opposed to interactive session, although a bit skeptical of its value considering my limited knowledge. I am in Pacific time zone.

[menkej]

I started working on this. I'm already down to only 15/50 tests failing. Some patches on base lib and also icloudpd needed. Quite some work left. Problem is, that the changes to pyicloud are also changing some base behaviour there... We'll see how the final pull request will look like...

[patrislav1]

Hi, is there anything going on in this direction (or any remains of said work that can be picked up on)? I noticed that pyicloud_ipd depends on an ancient version of keyring which breaks on Python3.10, so it can't be used on current systems w/o deadsnakes & virtualenv.

I'd be willing to help migrating this project to upstream pyicloud but first off I'd like to know if there is any work that can be built upon. I noticed there's a original_pyicloud branch, but there only the dependency has been switched from pyicloud_ipd to pyicloud w/o any further changes in the code (and it's already 2 years old).

[AndreyNikiforov]

I noticed that there are a number of PRs that require some changes to pyicloud. That project has not been updated for a year. I am debating (with myself) about dependency of icloudpd project on that lib:

1. remove the fork and rely on pyicloud
2. continue with the fork and add changes to it, e.g. for supporting sharedLibs
3. sync the fork from upstream, apply changes we rely on, and keep it as a buffer when our iteration is faster that pyicloud
4. totally consume pyicloud into our codebase
5. rewrite http request generation and parsing for photos in our codebase

There are pros and cons with each, as usual, and most of them not about one-time effort. My gut feeling is most radical at a first glance (rewrite) may be the easiest... I need to get deeper into the code to evaluate.

[AndreyNikiforov]

I'd be willing to help migrating this project to upstream pyicloud but first off I'd like to know if there is any work that can be built upon. I noticed there's a original_pyicloud branch, but there only the dependency has been switched from pyicloud_ipd to pyicloud w/o any further changes in the code (and it's already 2 years old).

I looked at syncing with upstream 2 years ago -- could not figure out good path within reasonable time. ;-( pyicloud had some changes since then, so there is a chance the gap shrunk.

[torarnv]

Why was this closed as completed? What was the outcome?