mirror of
https://github.com/koel/koel.git
synced 2026-04-25 08:46:00 +03:00
[GH-ISSUE #123] Add a progress bar during the import of songs #87
Labels
No labels
Authentication
Dependencies
Documentation
Feature Request
Flac
Help Wanted
Installation/Setup
Integration
Mobile
PR Welcome
Pending Release
Performance
Playlist
S3
Search
Sync
[Pri] Low
[Pri] Normal
[Status] Keep Open
[Status] Needs Author Reply
[Status] Needs Review
[Status] Stale
[Status] Will Implement
[Type] Blessed
[Type] Bug
[Type] Duplicate
[Type] Enhancement
[Type] Help Request
[Type] Question
[Type] Task
pull-request
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/koel-koel#87
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @AntoineTurmel on GitHub (Dec 20, 2015).
Original GitHub issue: https://github.com/koel/koel/issues/123
A progress bar would be nice to have an idea at the current progress when importing songs :)
@phanan commented on GitHub (Dec 20, 2015):
Not very high on my list though. Feel free to send a PR over ;)
@ronilaukkarinen commented on GitHub (Jan 22, 2016):
Any way to see a verbose output of
php artisan koel:sync? I have 55 invalid files and no idea which ones those are. Also a massive music library which takes over one hour to sync.@phanan commented on GitHub (Jan 22, 2016):
For right now you can go with -v, but I doubt it would help much if you
have that big a library. I'll take this into account to improve the sync
action - maybe a log to be generated.
On Fri, Jan 22, 2016 at 6:32 PM, Roni Laukkarinen notifications@github.com
wrote:
@X-Ryl669 commented on GitHub (Aug 16, 2016):
See #405 for a more verbose output.
@X-Ryl669 commented on GitHub (Aug 17, 2016):
I'm trying to implement a progress bar, and this is what I intend:
Are you ok with this ?
@phanan commented on GitHub (Aug 17, 2016):
No, I'm not :) Don't see how your solution, when pretty complicated and prone to problems, benefits the users.
@X-Ryl669 commented on GitHub (Aug 17, 2016):
When you have a large library, and click the "Scan" button in the interface, it fails.
It fails because the PHP script is running out of time (and you can not change this beforehand since you don't know how long it's going to take). So the idea is to split the sync's "big task" for a large library in smaller task, and let each of them report their progress. In reality, it's always the same task repeated as required to cover the whole library.
This way, the system is responsive, since it's the client's browser that's triggering each subtask (which are just Ajax calls that are serialized BTW).
In the end, the user see what's happening as if it was local with a nice progress bar, instead of a "Please wait" that fails after some time on with no reason shown except "Error 0".
@phanan commented on GitHub (Aug 17, 2016):
If the user has a large library, just use the console sync command, as recommended in the wiki. "Splitting" the task into 30-song chunks means 300 requests for 9,000 songs (a medium-size lib), which is far from optimal.
@BernardGoldberger commented on GitHub (Aug 17, 2016):
I would first want something in the command line that would tell me where the sync is up to, maybe
Scanning now (50/10,000)music detailScanning now (61/10,000)music detailetc.
In the command line, I believe getting status would be much easier.
You could use
Finderto get a file count and then count the files as they are being scanned.@phanan commented on GitHub (Aug 17, 2016):
@bdgold Or we can use a progress bar.
@BernardGoldberger commented on GitHub (Aug 17, 2016):
@phanan would it be visible with
-v?@X-Ryl669 commented on GitHub (Aug 17, 2016):
Well, 30 is an example. It can be 300, or more.
If the function is not intended to be used by the GUI part, then we should remove the "Scan" button once for all.
Right now, even with a 2000 songs library (small by your opinion), it fails to run in 60s (which is the default PHP runtime).
I'm not really fond of having to ssh on the server every time I add a song to the library to trigger the sync.
So, in the end, we could do this:
In all cases, you'll get a feedback, which is currently lacking from koel's GUI. Any kind of remote work from JS to PHP requires request anyway to get the progress status, so you'll can't avoid them.
@phanan commented on GitHub (Aug 17, 2016):
Just increase the time limit then…
Scanning is incremental, so adding new songs shouldn't rescan the whole library.
Anyway, I do admit that the current scan GUI lacks a feedback. But then, I don't think your solution is the best answer we can get. I'm waiting for Laravel 5.3 with Echo to be out, and then we will have cool real time stuff ;)
@phanan commented on GitHub (Aug 17, 2016):
@bdgold Like this:
@X-Ryl669 commented on GitHub (Aug 17, 2016):
To what ? 5mn ? 1 hour ? And if I do so, I'll have lingering scripts on my server. No, I'm not convinced.
I did not know about this, and the demo is nice. However, it means having a Websocket listening server (it can't be nginx+fpm/php, because of the time limit for the script). And again, in all cases, you'll still have many request from JS to PHP (even if they are hidden by echo).
@phanan commented on GitHub (Aug 17, 2016):
Not really, no. Echo uses Pusher (and other drivers as well to be implemented IIRC), so it's push, not pull. Quite a difference there.
@X-Ryl669 commented on GitHub (Aug 17, 2016):
Still, it's using Websocket underneath so it means no nginx+fpm, and firewall config to open the port for the websocket (because port 80 and 443 are using by nginx), + a process for serving the websocket (likely with php). And it's still as many message down from PHP to JS as there are songs, because the "new song scanned" event will be triggered for each.
No sure it's worth the trouble for the "Scan" feature.
Right now, it's broken in the GUI, and a simple solution would fix the need, that could be reworked whenever a good alternative with echo and Laravel 5.3 are out.
@phanan commented on GitHub (Aug 17, 2016):
I don't know if it's just me, but I'm not sure what all of these have to do with Laravel Echo and Pusher. If you haven't, I'd suggest reading Matt Stauffer's article here.
No, it won't. It will be triggered in batches, which can be calculated easily and efficiently from the server side.
First, there shouldn't be any trouble. Second, I have plans to use Echo/Pusher anyway, for other features.
I honestly don't think it's any broken to start with. It can use some improvements, yes, which unfortunately I don't see in your solution. Let's not take this further.
@X-Ryl669 commented on GitHub (Aug 17, 2016):
I've tried Koel because it's a independant PHP + JS solution that can be installed on my already PHP + nginx server (and it looks nice). As soon as the code will require either a external API (yes I'm speaking of Pusher which is not free for large amount of calls, and it means stop being independant) or a live websocket, it'll be a PITA to use, and I'll have to stop using it, because WS requires a long running process to stay on my server, and many headache to configure firewall or reverse proxy which I don't want to have.
Anyway, you're right it's out of the scope of this issue, so I'll stop here.
@X-Ryl669 commented on GitHub (Sep 1, 2016):
Please find an implementation of what you don't like (but works until the solution with bells and whistles is there): https://github.com/X-Ryl669/koel