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

Add summary mode to API /serie/byfolder for Web UI #619

Closed
hidden4003 opened this issue May 12, 2017 · 36 comments
Closed

Add summary mode to API /serie/byfolder for Web UI #619

hidden4003 opened this issue May 12, 2017 · 36 comments

Comments

@hidden4003
Copy link
Member

hidden4003 commented May 12, 2017

@japanesemediamanager/api

WebUI Import folders tab was using API /serie/byfolder to show list of series in a folder with their filesizes, series strorage was calculated as total of all videolocals.

Current version has no filesizes and returns a lot of stuff I don't need.
I don't know which is better adding a mode here or making new api call.

@da3dsoul
Copy link
Member

I'd say we should make a new module targeting stats. It'll have minimal info, but stats tend to have separate use case with speed taking priority. Having a separate module targeting it would reduce the code complexity.

@bigretromike
Copy link
Contributor

/serie/ should return info about serie. current thing should show file size with proper level deep, but that would also give alot of info about serie. You probably want something like ls or dir command.

What info you need to be return so we can thing of filter for current api or add new command to list series in folders.

But probably like @da3dsoul new module for stats could be good idea as it would be speed over amount of info. the main question would be how fast will those command runs, as we could end up iterating by same objects on both api's ending up with same speed...

@hidden4003
Copy link
Member Author

Web UI has import folders tab where it shows a list:
Import folder (select at the top)
Series name - 1.23 GB
Series name2 - 500MB

What can be improved is probably to show number of videolocals vs total number of videolocals. I am thinking about a series being split into two separate foldes so you would see something like:

Series name (20 files of 35) - 1.23 GB

Web UI screeen I am talking about
image

@da3dsoul
Copy link
Member

Just make a list of exactly what data you want, and I will make you endpoints for it

@da3dsoul
Copy link
Member

Preferably in tree form

@hidden4003
Copy link
Member Author

An array of Series each having:

  • Title
  • Total number of files (videolocals) in all import folders
  • Number of videolocals in selected folder
  • Total filesize in bytes (sum of Videolocal.Filesize fields) in selected folder

@bigretromike
Copy link
Contributor

You want series. So we looking for Serie based on folder you select. Basicly to calculate size of serie you need to know what files you have there, you need Episodes of that Serie having all Episodes you need all RawFiles to have them you need to be on level=2 so basicly it will take same time as api/serie/byfolder?level=2 . Just saying. Until some db rework (or that factory thing) it wont be faster;

@hidden4003
Copy link
Member Author

Added in ShokoAnime/Shoko-WebUI@5e45a51

Cazzar pushed a commit to maxpiva/ShokoServer that referenced this issue Feb 7, 2018
@da3dsoul da3dsoul reopened this Feb 27, 2018
@da3dsoul
Copy link
Member

This broke at some point.

@hidden4003
Copy link
Member Author

Performance is very bad, for my collection (3890 files, 246 series) it took 2 minutes.

image
image

@ElementalCrisis
Copy link
Member

It's also supposed to show number of files for that series. Old mockup but should get the point across.

import folders - mockup

@bigretromike
Copy link
Contributor

@hidden4003 its slow because its simple iteration over vlocal folde, serie inside folder, episode inside serie; this was needed because you wanted size of each serie in that folder; without pre-populating static values like if you add/remove series some static table would get updated so you get almost 'identical' results in just a manner of miliseconds you need to iterate over it all the time you want the info - or we could just that info to vlocal folder so if you query folder the folder know its 'content' and that would work also for cloud based/external drives which would be fun. but then again we would have to iterate over vlocal series to get episode or if we store local count of files we could get that.

@ElementalCrisis you are probably right, but that was never the case - that function never did return those information;

@da3dsoul how did it broke? I query it and its all the same as it was when added - ultra slow, but working;

@Cazzar
Copy link
Member

Cazzar commented Feb 28, 2018

ultra slow is kinda an understatement:

[cayde@aoki-lapis ~]$ time curl -s -H 'Accept: application/json' -H "apikey: $apikey" 'http://localhost:8111/api/serie/infobyfolder?id=2&limit=999999' | jq '. | length'

700

real    21m11.824s
user    0m0.000s
sys     0m0.050s

@bigretromike
Copy link
Contributor

@Cazzar iterating over your/mine collection give those results. thats why by default its limit=100. also it was added as container because no-one cared enought to add this and we wanted to push webui forward. also abotu webui. fix that docker of your because its is not compatible with wizard which would help with configuration alot.

To be honest, You can see that I wrote Until some db rework (or that factory thing) it wont be faster;
And no one cared enough 👍 - also Im not ShokoDB expert maybe some of needed data is already stored somewhere. I added mockup function that iterate and give real-life-time data about folder which is super slow on big collection. Simple as that. We can use paging or rewrite function or rewrite function with db table updates. Well free to pick one

@da3dsoul
Copy link
Member

And then we'll see my rewrite cut it down to milliseconds

@hidden4003
Copy link
Member Author

Query like this

select sum(v.FileSize)/1048576, COUNT(*), cf.AnimeID, vu.JMMUserID from VideoLocal_User vu, VideoLocal v, VideoLocal_Place vp, CrossRef_File_Episode cf where vu.VideoLocalID = v.VideoLocalID and v.VideoLocalID = vp.VideoLocalID and vp.ImportFolderID = 1 and v.Hash = cf.Hash group by vu.JMMUserID, cf.AnimeID

result seems to make sense at least its close:
image

against my SQLite DB it returns almost instantly so it is definitely possible, sure it is not all info, only sizes and counts, series name calculation is complex but those objects are cached and it probably won't be 21 minutes.

@da3dsoul
Copy link
Member

I'm not going to use custom SQL just yet, since max is refactoring DB stuff

@Cazzar
Copy link
Member

Cazzar commented Feb 28, 2018

The issue itself I believe may revolve around

Serie ser = Serie.GenerateFromVideoLocal(Context, vl, uid, true, true, 2, false, false, 0, tagfilter);

@bigretromike
Copy link
Contributor

@da3dsoul you are some kind on ninja-coder 👍
@hidden4003 yeah, not as trivial but as I recall we dont use sql query because we use NHibernate to make those
@Cazzar this and the fact its iterate over each row, but @da3dsoul know some shinigamis tricks to profile those xD as usually
@da3dsoul didn't he refactored it before ? is there end with those refactoring ?:P maybe we should have "Make full documentation before of refactoring DB before applying" that would stop most people from refactoring thing that often. I know he does it with good intentions.

@Cazzar
Copy link
Member

Cazzar commented Feb 28, 2018

This has been resolved by 8a54767

[cayde@aoki-lapis]$ time curl -s -H 'Accept: application/json' -H "apikey: $apikey" 'http://localhost:8111/api/serie/infobyfolder?id=2' -o /dev/null

real    0m0.131s
user    0m0.000s
sys     0m0.000s

though @bigretromike I do know how to do profiling as well, and it turned out that my suspicions were true, generating the contract for every episode... not that clean of a way to do it to begin with.

@da3dsoul
Copy link
Member

Reclosed

@bigretromike
Copy link
Contributor

8a54767 does it respect user access levels (aka Family friendly etc),
also the limit arg was removed preventing from pagining support.
@da3dsoul could you re-add those ? I see we missed offset there for full pagining;

@bigretromike
Copy link
Contributor

@Cazzar I also dont know how profiling works - but I understand that its better to add slow but working code just because it will help to develop in this case webui than to don't add nothing and make project stale; And as always @da3dsoul did not failed in making it stupidly-fast;

@da3dsoul
Copy link
Member

It doesn't need paginating. It's a stats listing. It also doesn't respect user levels, it's a stats listing

@bigretromike
Copy link
Contributor

so it way to bypass collection restrictions then ?
also I why would you want to throw 2000 or 50000 series list onto webui/api client when there is space for 5, 10 rows?

@da3dsoul
Copy link
Member

da3dsoul commented Mar 1, 2018

No it's just the purpose of it is to list very little info on everything for totals

@bigretromike
Copy link
Contributor

are we talking about same api endpoint ? it return series inside give import folder.
name of serie, size, ep count. that not 'very little'. I easily hit few 100 of items in one folder.

@bigretromike
Copy link
Contributor

Ok, if it is to much I will do it later myself, no point it pushing this so long.

@da3dsoul
Copy link
Member

da3dsoul commented Mar 1, 2018

It gives exactly what he asked for in the OP, nothing more except ID.

@da3dsoul
Copy link
Member

da3dsoul commented Mar 1, 2018

Please don't touch it, it works great

@da3dsoul
Copy link
Member

da3dsoul commented Mar 1, 2018

It's serie/infobyfolder

@bigretromike
Copy link
Contributor

then please add limit and offset

@da3dsoul
Copy link
Member

da3dsoul commented Mar 1, 2018

Nope it's getting changed as soon as max merges his latest project anyway. We are starting APIv3 at that point. Making it more standardized

@da3dsoul
Copy link
Member

da3dsoul commented Mar 1, 2018

No one except avael uses it, so I'm not concerned

@bigretromike
Copy link
Contributor

if no one use it.. why did you care to fix it in first place?

@da3dsoul
Copy link
Member

da3dsoul commented Mar 1, 2018

"No except avael" because we love avael, and I'll fix things for the future of WebUI and ClientV2

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

No branches or pull requests

5 participants