Rework how we fetch community plugin data #2222
Conversation
Introduces the concept of a store of community plugins, which is currently limited to the official PluginsManifest repository. Each store can support more than one manifest file URLs. When fetching, all URLs are used until one of them succeeds. This fixes issues with geo-blocking such as Flow-Launcher#2195 Plugin stores can be expanded in the future to be user-configurable, see Flow-Launcher#2178
avoid repeatedly fetching manifest data while the user is typing a `pm` query
from a `pm install` or `pm update` query should help with Flow-Launcher#2048
JohnTheGr8
added
enhancement
| Settings.InstallCommand => await pluginManager.RequestInstallOrUpdate(query.SecondToEndSearch, token, query.IsReQuery), | ||
| Settings.UninstallCommand => pluginManager.RequestUninstall(query.SecondToEndSearch), | ||
| Settings.UpdateCommand => await pluginManager.RequestUpdateAsync(query.SecondToEndSearch, token), | ||
| Settings.UpdateCommand => await pluginManager.RequestUpdateAsync(query.SecondToEndSearch, token, query.IsReQuery), |
There was a problem hiding this comment.
why requery means use the primary url?
There was a problem hiding this comment.
I see. I previously thought Last() refers to cdn.
| internal async ValueTask<List<Result>> RequestInstallOrUpdate(string search, CancellationToken token, bool usePrimaryUrlOnly = false) | ||
| { | ||
| await UpdateManifestAsync(token); | ||
| await PluginsManifest.UpdateManifestAsync(token, usePrimaryUrlOnly); |
There was a problem hiding this comment.
If we fetch result every time, the ctrl+r will only refresh the result on the first one, but later one will still use the manifest in cdn? I believe we should have local cache for this. (Inserting another character will revert back to the old results).
There was a problem hiding this comment.
We don't fetch every time, I have added a 10-second timeout so that we don't fetch repeatedly while a user is typing. Hitting Ctrl+R will bypass the timeout and fetch from the primary url.
Inserting another character will revert back to the old results
so this is only an issue if the user waits 10 seconds before inserting that extra character.
There was a problem hiding this comment.
10 seconds seems to be pretty short? As we have the ctrl+r features, why don't we add a couple minutes timeout?
There was a problem hiding this comment.
10 seconds seems to be pretty short?
it's only meant to avoid multiple http requests while a user is typing
As we have the ctrl+r features, why don't we add a couple minutes timeout?
I don't see a reason to: fetching is very fast with the CDN links, and I've added 3 CDN links now so that fetching should be fast for users anywhere in the world (the fastest cdn link will be used in each user's case).
It takes around 80ms for me, a 2-minute timeout for a call this quick seems a bit unreasonable 😂
There was a problem hiding this comment.
If you are in China, the cdn is not so fast. It takes around 1s to download the manifest for me without proxy (with proxy it shall be fast).
the other cdn takes 600ms, but sometimes it doesn't work.
There was a problem hiding this comment.
Okay, I can increase the timeout to a couple minutes. Does the rest of the PR look good to you?
the other cdn takes 600ms, but sometimes it doesn't work.
yeah that's served by cloudflare, don't think they're very solid in China
There was a problem hiding this comment.
The rest looks good to me. I will take a detail look today. Previously jsdelivr has license in China, so they are able to deliver cdn that located in China, which is much much faster and stabler.
taooceros
left a comment
taooceros
left a comment
There was a problem hiding this comment.
Sorry for the delay. It looks very good to me.
|
@jjw24 do you also wanna have a look before we merge? |
|
Yep, I will take a quick look as well then merge. |
3 participants
Community Plugin Stores
Added an abstraction for community plugin stores, which can be configured with one or more manifest file URLs (for example, a Github link and a few CDN links). When fetching plugin data, all manifest file URLs are used until one of them succeeds. This fixes issues with geo-blocking such as #2195 without compromising performance.
For the moment we only have one plugin store (the PluginsManifest repository), but this concept can be expanded later to be fully customizable and to support unofficial plugin communities (see #2178)
Plugin manager enhancements
Using
Ctrl+Rfrom apm installorpm updatequery will result in fetching plugin data from the primary manifest URL, which is a github link. This will bypass any configured CDN links and force Flow to get the latest data, which helps with #2048PS: I had a very different approach to #2200 in mind, and thus decided to open another PR instead of trying to explain