search

LycheeOrg / Lychee

A great looking and easy-to-use photo-management-system you can run on your server, to manage and share photos.
https://lycheeorg.github.io/
MIT License
3.34k stars 299 forks source link

[TO BE STACKED] Support of S3 as an alternative storage backend (#149) #2281

Closed ildyria closed 5 months ago

ildyria commented 8 months ago

2208 but on top of pipes. :)

codecov[bot] commented 8 months ago

Codecov Report

Attention: 73 lines in your changes are missing coverage. Please review.

Comparison is base (8d68a27) 86.67% compared to head (6e8b31e) 86.30%.

Additional details and impacted files
ildyria commented 8 months ago

@Kovah gave you write access to the repo, you should be able to take over that branch. :)

As follows are the change I made:

Kovah commented 8 months ago

Thanks! Will have a look in the next days.

Kovah commented 7 months ago

So, I added the file deletion changes I had locally. Instead of paths, the size variants are pulled as model entries with only the path and disk information.

There are two issues:

ildyria commented 7 months ago

Regarding live photos: it seems they are stored beside the photo. That's why I added the storage disk to the photo model too. Otherwise, there is currently no way to determine the storage location for live photos. What do you think?

I would consider it differently and use the position of original to determine where the video of SizeVariant is. The other possibility is to first check if the file exists on Local, if not try with s3, but ideally it should be on the same disk as original.

ildyria commented 7 months ago

In the Album/Delete.php action, album tracks are added to the file deleter (s3-pipes/app/Actions/Album/Delete.php#L110) after the deleter was run. Can this be removed? Not sure if they are used for something.

I would approach it a different way, use the method Album::deleteTrack() and not bother further with it. :) Tracks are not used per say in the v5, but that is because I haven't had time to implement the functionalities.

ildyria commented 7 months ago

Also a few nice tips.

Kovah commented 7 months ago

Fixed code formatting. Not sure how you want to implement the album track deletion if it's not really used right now.

My issue right now is the immense complexity of the application and that I can't spend a lot of time digging into it. Sorry. I tested the S3 handling locally and everything seems to work, including temporary URLs if symlinks are enabled. File deletion seems to work properly. I think a large potion of the S3 feature is done.

I hate to say this, but I guess the completion of the feature is up to you. I don't feel confident in getting the whole picture right.

ildyria commented 7 months ago

My issue right now is the immense complexity of the application.

I am sorry about that. :(

Not sure how you want to implement the album track deletion if it's not really used right now.

I will investigate. No worries. :)

I tested the S3 handling locally and everything seems to work, including temporary URLs if symlinks are enabled. File deletion seems to work properly. I think a large potion of the S3 feature is done.

Great to hear. I don't use S3 so I can't test it properly.

I hate to say this, but I guess the completion of the feature is up to you. I don't feel confident in getting the whole picture right.

No worries. I'll take it from there then. :)

Do note that due the limited number of people in the team, this PR may take while to hit master. :) That being said thank you for your help!

ildyria commented 7 months ago

I hate to say this, but I guess the completion of the feature is up to you. I don't feel confident in getting the whole picture right.

I should have some time to take a look at it now. :)

Kovah commented 7 months ago

Let me know if I can help test stuff. Have both Minio and Cloudflare R2 at hands.

ildyria commented 7 months ago

@Kovah can I trouble you to test the latest code?

Please have PHOTO_PIPES=true in your .env

In theory the videos should now be uploaded to s3 too.

If you can also try with the following two: https://github.com/LycheeOrg/Lychee/blob/master/tests/Samples/train.jpg https://github.com/LycheeOrg/Lychee/blob/master/tests/Samples/train.mov

First uploading the jpg then the mov. Then (after deleting) uploading the mov then the jpg. This would test the 2 code paths.

And also the following: https://github.com/LycheeOrg/Lychee/blob/master/tests/Samples/google_motion_photo.jpg

Kovah commented 7 months ago

Test was not successful. Uploaded train.jpg and gut this error:

[2024-02-20 16:03:21] production.ERROR: App\Models\Photo::toArray:28 livewire.update:App\Models\Photo->toArray() is deprecated, use Resources instead.
App\Models\Photo::jsonSerialize:118 App\Models\Photo::toArray() failed; caused by

No more detail in the log files, unfortunately. But the original was uploaded to S3. When trying to upload it again, I get another error.

[2024-02-20 15:50:29] dev.ERROR: App\Actions\Photo\Pipes\Duplicate\ReplicateAsPhoto::handle:13 Call to a member function replicate() on null

Side note: something is heavily messing with the log files. There are thousands of lines added with random request details and json-formatted log entries not readable at all.

ildyria commented 7 months ago

Thanks for the feedback!

App\Models\Photo::toArray:28 livewire.update:App\Models\Photo->toArray()

I will look into that.

[2024-02-20 15:50:29] dev.ERROR: App\Actions\Photo\Pipes\Duplicate\ReplicateAsPhoto::handle:13 Call to a member function replicate() on null

I will look into that too.

Side note: something is heavily messing with the log files. There are thousands of lines added with random request details and json-formatted log entries not readable at all.

My guess is that the connection to AWS is resulting in that.