Yooooomi / your_spotify

Self hosted Spotify tracking dashboard
GNU General Public License v3.0
3.02k stars 122 forks source link

Improve global request performances #354

Closed quentinguidee closed 7 months ago

quentinguidee commented 7 months ago

Benchmarks: https://github.com/Yooooomi/your_spotify/pull/354#issuecomment-1973566720

Yooooomi commented 7 months ago

I don't know what you think about the userMetric thing in the app. I think it's not available anymore but it used to allow people to choose between choosing from "Count" or "Duration". It makes frontend more complex and requests more complex too so maybe we should stick to one or always do both. I'd be curious to know what you think :)

quentinguidee commented 7 months ago

Actually in my opinion that's something to consider and to keep since both are useful and different statistics. For now for this PR I'll keep the requests as they are, because each deep modifications of the requests should probably have their own PR and discussions. I'll dedicate this PR to late $lookup first!

Also, I think that this filter can be more pertinent in places like /top/tracks.

Ultimately, the /top/tracks route might also be better as just /tracks with filters/sort capabilities:

I think this approach might be is more RESTful

Yooooomi commented 7 months ago

Yeah I agree with you but in the end they define a use case that has to be present in the app. I feel like query flexibility should be linked to frontend graph flexibility. As of today you cannot customize graphs much so there's not real need for that.

quentinguidee commented 7 months ago

For this specific frontend yep, but anyone that want to use the API for its own frontend can't really use it except if it reproduces the exact same views. If I want to make a small utility or script that uses my stored listening history, I cannot do that without editing the server (except if the queries are used by the yourspotify frontent).

Yooooomi commented 7 months ago

Ok yeah I can understand, I just feel like we should put minimal effort on this on our way to performances.

quentinguidee commented 7 months ago

Artists/Albums/Tracks pages and ranking are instantaneous now 🚀 That's really cool to navigate

Yooooomi commented 7 months ago

Hey, really nice work! However I can't seem to find the functions getRank, getBest and the itemTypes enum? Is there anything I'm missing?

quentinguidee commented 7 months ago

I added this hidden page so we can benchmark more easily requests separately. So that's the current state:

Before this PR

image

After

image

Also, requests for artists/tracks/albums are not shown yet

Yooooomi commented 7 months ago

Looks awesome, I love stats and numbers haha! Do the requests follow the interval given on top? Are you ready for a merge?

quentinguidee commented 7 months ago

Yeah the intervals are from the header!

Yep it's ready. There are still improvements that we can do but that'll be enough for this one!

Yooooomi commented 7 months ago

Hahaha

image

I'm fixing that dont worry

Yooooomi commented 7 months ago

Are you working on anything specific right now? I feel like I could give some requests a try too.

quentinguidee commented 7 months ago

No, everything for now was in this pr 👍

quentinguidee commented 7 months ago

Also after this I think you can mark #162, #232 and #323 as resolved. There are also a lot of issues that could be closed

Yooooomi commented 7 months ago

You inspired me #355 haha