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

fix: missing album/artist/track in database #430

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

M2K3K5
Copy link
Contributor

@M2K3K5 M2K3K5 commented Sep 7, 2024

fixes #429 by re-fetching missing album, artist, or track data from Spotify when it is not found (or invalid) in the database. This ensures that the relevant data is added to the database and prevents page failures or repeated requests.

@Yooooomi
Copy link
Owner

Yooooomi commented Sep 7, 2024

Hello! This is something that is already done at startup. If the behavior at startup does not work, this is the thing to fix. I feel like there is a lot of overhead on basic database requests with your pr.

@M2K3K5
Copy link
Contributor Author

M2K3K5 commented Sep 8, 2024

@Yooooomi : I've added a fix for your startup feature to fix the missing artist issue. I also fixed issue #309 by updating the getHistorySongs endpoint to skip problematic tracks that were causing the dashboard to crash due to missing artist/album data.

However, I want to emphasize that fixing the database only on startup is not an ideal long-term solution, since most users don't restart their processes often. For example, if I weren't actively developing the project, I would probably only restart it every few months or when a new version is released.

My new code provides a more dynamic solution-it is designed to handle missing artists during regular operation. From the user's perspective, this process should be seamless, and if an artist isn't found in the database, the system will avoid crashes or errors. The only downside is a slight delay (about a second) during data fetch, but this trade-off significantly improves stability and would only happen if the artist/album is missing in the database (which should not happen in the first place).

@Yooooomi
Copy link
Owner

Yooooomi commented Sep 8, 2024

If we want to add a solution, it has to be seamless. Seamless in the duration of the request but also, as you said, at runtime to fix any possible issue. I feel like you're building fixes on top of features and it should not be this way. Features should work in isolation without knowing if there is behavior fixing the database. If we think about a solution, it has to be done on the side of the current project. Thanks a lot, really, for your work. And I hope you can understand my point of view.

@M2K3K5
Copy link
Contributor Author

M2K3K5 commented Sep 8, 2024

I see your point. Would you prefer moving the re-fetch logic to a different part of the code, outside of the ArtistModel.find function? Another alternative could be executing fixMissingTrackData as a nightly task.

The issue with only fixing the database at process start is that if a single entry is broken, it will break the entire dashboard and the top sessions for the user. To prevent this, the database re-fetch needs to happen as soon as possible, in my opinion.

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

Successfully merging this pull request may close these issues.

[Bug] "Dashboard" and "Top Sessions" break when artist is missing in database
2 participants