FreeTube
FreeTube copied to clipboard
FreeTubeApp
Don't fetch additional data when importing subscriptions
Don't fetch additional data when importing subscriptions
Pull Request Type
- [x] Bugfix
- [x] Feature Implementation
Related issue
- Avoids rate limit when importing subscriptions
- closes https://github.com/FreeTubeApp/FreeTube/issues/3943
Description
This PR will load the subscriptions as is and will load data only when needed (thumbnails will be slowly loaded in the side bar if channels are set to be displayed there). If someone disables the sidebar and doesn't use the Subscribed Channels page then thumbnails will be added when a user navigates to a channel.
Testing
See: https://github.com/FreeTubeApp/FreeTube/issues/3735#issuecomment-1642286050
- profiles.txt
- subscriptions.csv
- SubscriptionsError.csv
- SubscriptionsNoError.csv
- Test.Subscriptions.txt
- freetube-subscriptions.json
Change file extension .txt to .db
(Notice how much faster importing subscriptions is)
Desktop
- OS: Windows
- OS Version: 10
- FreeTube version: 0.19.1
issue (blocking): The tooltip text could use a bit more clarity on the implications of the choice & what exactly is at stake (pros and/or cons). As a user, the tooltip opens up more questions for me than it answers.
question: I'm also missing some context on the intention of this change. Is this more of a temporary workaround to that issue, a longer-term workaround because that issue has no clear fix, a way to reduce requests that is worthy as a feature on its own merits, or some mix of the three? A clearer tooltip as mentioned above would clear up my confusion.
issue (blocking): The tooltip text could use a bit more clarity on the implications of the choice & what exactly is at stake (pros and/or cons). As a user, the tooltip opens up more questions for me than it answers.
question: I'm also missing some context on the intention of this change. Is this more of a temporary workaround to that issue, a longer-term workaround because that issue has no clear fix, a way to reduce requests that is worthy as a feature on its own merits, or some mix of the three? A clearer tooltip as mentioned above would clear up my confusion.
The main purpose is to avoid rate limiting when importing subscriptions from other sources (invidious, newpipe, youtube) but disabling the toggle also fixes other issues for channels that are age restricted, deleted, etc. as we handle that logic on the subscription page already. It also greatly improves the speed of importing subscriptions.
Now that I think about it, I wonder if I should just remove the prompt and not make any extra requests during the import at all.
One worry I have with adding the thumbnail fetching to the side bar, is that we might just be moving the ratelimiting problem to the side bar. #4231 will make the problem worse though as it allows you to display up to 5 columns of channels in the sidebar at the same time.
Another option instead of doign nothing is doing a much smaller number of requests e.g. maximum 50 requests, that way it won't get ratelimited but it'll still be able to show thumbnails for a bunch of channels straight away.
Also it should probably be made more robust so it can actually skip deleted/removed channels without aborting the whole import process.
If we can avoid fetching data that would probably be best.
Sounds good, I'll most likely update it this weekend to avoid fetching the data
Could this fix https://github.com/FreeTubeApp/FreeTube/issues/3211#issuecomment-1439345844?
Sounds like it would introduce the same issue to the side bar, that said comment mentions on the subscribed channels page, the the logic to fetch the thumbnails from the API when it's missing was copied/inspired from there.
Could this fix #3211 (comment)?
I can't seem to reproduce that error, so it might be fixed or it might be a bit more complex
issue (very minor, non-blocking): When the channel thumbnail fails to load, it still shows something ugly for a few seconds until it corrects itself.
The thumbnail failing to load is what's used for FreeTube to correct itself, I'm not sure if there's a better way to handle it than by failing to load an image and then making a request to get an image when it fails
Right now it seems that the new version relies on invalid url and error handler to update thumbnail URLs To properly fix this a default image (URL) needs to be displayed and a handler (on show?) to conditionally update the thumbnail Not sure if it's quick to implement
Default image should now be shown when importing and constants are now used
If this is meant to address the issue I mentioned earlier, still seeing it:
Still fine if this goes unfixed as it's pretty minor, just wanted to communicate that.
I don't know how to see loading-spinner icon at all If I am offline the side bar just shows images as missing
How I test offline
Side nav looks the same as before, but everything seems to be working fine. Still haven't seen the loading spinner or the profile font-awesome-icon.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Conflicts have been resolved. A maintainer will review the pull request shortly.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Conflicts have been resolved. A maintainer will review the pull request shortly.
- Isnt this a bit user unfriendly behavior? User needs to go to every channel to make the loading icons disappear. With this example it isnt a big task but with 100+ that becomes quite a chore
https://github.com/FreeTubeApp/FreeTube/assets/73130443/b35b9c1c-08b5-4dda-9eda-ae29214b86f1
- why no loading icon here?
https://github.com/FreeTubeApp/FreeTube/assets/73130443/d2eadacb-b628-40c6-a30b-5b3a8e422c11
Test files work for me (see loading icon) But should that work for FT sub DB files too?
- Isnt this a bit user unfriendly behavior? User needs to go to every channel to make the loading icons disappear. With this example it isnt a big task but with 100+ that becomes quite a chore
VirtualBoxVM_atyrwtVaTk.mp4 2. why no loading icon here?
VirtualBoxVM_P5DAk8PIIE.mp4
You seem to have a lot of errors in the console, could you send a screenshot of them?
@ChunkyProgrammer apologies about the previous video, there seems to be a mistake on my end.
New testing has discovered the following:
- On clean startup channel icons load when imported but if you remove channels and import again channel icons wont load
https://github.com/FreeTubeApp/FreeTube/assets/73130443/89c201f7-de1b-48bf-8fa5-231adfa954c4
- After importing
freetube-subscriptions.json, most of the subs loaded fast but there a quite a few that are spinning forever
https://github.com/FreeTubeApp/FreeTube/assets/73130443/b9533036-4ffc-4e17-b1a9-203e29f4fb4a
- Is this still something that you want to address?
issue (very minor, non-blocking): When the channel thumbnail fails to load, it still shows something ugly for a few seconds until it corrects itself.
The thumbnail failing to load is what's used for FreeTube to correct itself, I'm not sure if there's a better way to handle it than by failing to load an image and then making a request to get an image when it fails
- FT doesnt seem to be happy about me visiting the profile page.
https://github.com/FreeTubeApp/FreeTube/assets/73130443/c4e60294-9439-40ee-8a2e-14c441272474
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Conflicts have been resolved. A maintainer will review the pull request shortly.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Conflicts have been resolved. A maintainer will review the pull request shortly.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Conflicts have been resolved. A maintainer will review the pull request shortly.
This pull request has conflicts, please resolve those before we can evaluate the pull request.