Update Package instead of whole package

[taooceros]

Question
For Update, do users have to download the full package including the .net core runtime? Is it possible to download a package only for update, which may be smaller.

By the way, I think the auto update and check for update doesn't work really well?

[jjw24]

Flow is self contained so the core run time are embedded. Downside to this is a larger install file at 80mb but this will help users who use it as portable.

It would be nice to just update with a smaller or actual Flow package but that won't be something I will have time to look at in the immediate short term myself.

The update process takes slightly longer, but has been reliably installing the updates, including using the auto update feature. Couls you expand on not really well?

[jjw24]

I guess the work around is publishing the non-self contained and having an option in Flow for users to pick, caters for users who have core installed.

A bit messy for the portable part though what if you take it to a machine without Core.

[taooceros]

The update process takes slightly longer, but has been reliably installing the updates, including using the auto update feature. Couls you expand on not really well?

Sorry for that I haven't seen the reply till now. In my computer, when I click "Check for Updates", it will give a hint that it is checking for update and please wait, but that's it. No more notice for whether there is a new update or not.

I am not sure whether the auto-update feature work well as intended since there is no notification of whether it is updating. I have manually installed the latest version of flow when it has released last time due to the weird notification.

I think the failure of checking update may be due to my personal internet issue, since I have checked the log, which indicating "Please check your connection and proxy settings to api.github.com". However, I do think that we should have a notice if the updater fails.

A bit messy for the portable part though what if you take it to a machine without Core.

I think we should let users who want to use the portable version to install a version with core, though it may be a bit messy ban the portable option for non-self-contained version. I think it should be ok to just put a warning at the release page suggesting the portable option may work not as intended if download the non-self-contained version.

I think wox doesn't worry about this issue? Is it because wox utilize the .net framework, which should be installed in most windows computer?

[jjw24]

Sorry for that I haven't seen the reply till now. In my computer, when I click "Check for Updates", it will give a hint that it is checking for update and please wait, but that's it. No more notice for whether there is a new update or not.

I am not sure whether the auto-update feature work well as intended since there is no notification of whether it is updating. I have manually installed the latest version of flow when it has released last time due to the weird notification.

Since version 1.2.0 there has been added messages to notify the update progress. Let me know if they are sufficient.

I think the failure of checking update may be due to my personal internet issue, since I have checked the log, which indicating "Please check your connection and proxy settings to api.github.com". However, I do think that we should have a notice if the updater fails.

Having a good internet connection definitely helps, 80mbs is not too big of a file to download. Have you ever been successful in updating via the client (auto or manually checking)?

I think we should let users who want to use the portable version to install a version with core, though it may be a bit messy ban the portable option for non-self-contained version. I think it should be ok to just put a warning at the release page suggesting the portable option may work not as intended if download the non-self-contained version.

There are probably many ways to tackle this. If we are to offer a non-self contained, we may need to publish two version every time and build in two ways to update the app in the code. I think one of the better ways is able to separate out the Core runtime installation, user can choose only to install it when manually or automatically when not present. This obviously requires a bit of work to do though.

I wonder if we can lighten the Core runtime packages, so the installer is much smaller, even if we can half the size will be good. I will look to do this first actually.

I think wox doesn't worry about this issue? Is it because wox utilize the .net framework, which should be installed in most windows computer?

That's right. .Net Framework runtime comes pre-installed with Windows machines so there is no need to install it on most occasions. However development wise it's heading towards .Net 5 so being on Core is the most idea situation for Flow as we want to see continued development with a supported framework.

[taooceros]

Having a good internet connection definitely helps, 80mbs is not too big of a file to download. Have you ever been successful in updating via the client (auto or manually checking)?

Well, since I started using flow since version 1.2.0, I haven't been successfully update through auto & manually checking. I manually download it and install it. I think the internet issue may be due to my internet connection to GitHub since I am in China or some proxy issue, though not sure.
Part of the log here.
2020-09-23 10:57:01.8048|ERROR|Updater.UpdateApp|-------------------------- Begin exception -------------------------- 2020-09-23 10:57:01.8048|ERROR|Updater.UpdateApp|Please check your connection and proxy settings to api.github.com. 2020-09-23 10:57:01.8048|ERROR|Updater.UpdateApp|Exception full name: <System.Net.WebException> 2020-09-23 10:57:01.8048|ERROR|Updater.UpdateApp|Exception message: <The operation has timed out.> 2020-09-23 10:57:01.8048|ERROR|Updater.UpdateApp|Exception stack trace: < at System.Net.HttpWebRequest.EndGetResponse(IAsyncResult asyncResult) at System.Net.WebRequest.<>c.<GetResponseAsync>b__68_2(IAsyncResult iar) at System.Threading.Tasks.TaskFactory1.FromAsyncCoreLogic(IAsyncResult iar, Func2 endFunction, Action1 endAction, Task1 promise, Boolean requiresSynchronization) --- End of stack trace from previous location where exception was thrown ---

I may look on the actual reason of this exception on my computer by cloning the code when I am free.

Since version 1.2.0 there has been added messages to notify the update progress. Let me know if they are sufficient.

Well, since I haven't successfully updated or checked for update, I haven't seen the message ever. My point is that it would be better to notice users that the update is failing, since I have checked the code, and when catching error, the updater will only provide a log but not giving message to user.

catch (Exception e) when (e is HttpRequestException || e is WebException || e is SocketException) { Log.Exception($"|Updater.UpdateApp|Please check your connection and proxy settings to api.github.com.", e); return; }

If we are to offer a non-self contained, we may need to publish two version every time and build in two ways to update the app in the code. I think one of the better ways is able to separate out the Core runtime installation, user can choose only to install it when manually or automatically when not present. This obviously requires a bit of work to do though.I wonder if we can lighten the Core runtime packages, so the installer is much smaller, even if we can half the size will be good. I will look to do this first actually.

Well I think we may not need to publish two version, but just notice users that the portable mode may not work as intended without core installed.
Installing core separated seems also a good idea.

Since .net 5 is going to release soon, and it will intergrate .net core and framework, I think microsoft may make it pre-installed with Windows just like what .net framework does. That seems will not be an issue in the future.

[taooceros]

I have manually checked the error with debugging. The exception is actually "The operation out of time", while when I move the timeout setting up to 3000, sometimes the request can work for me.
However, it is still unstable for me.

Since in our Http class, it utilizes the httpwebrequest to create request, I tried to change it to the new HttpClient, and it works fine. That's seems strange to me, and I have no idea why it can solve the exception.

Besides, at the time I am searching for the utility of HttpClient, there are quite a lot of discussion suggesting that we should not create httpclient instance every time to request. Instead, it should be created for once or get from IHttpClientFactory registered in service. I am not quite sure how that should be done in Flow. That seems may make it harder to change the Proxy setting comparing the implementation now.
It seems that we can just change the global setting of proxy to change proxy setting. However, this require testing since it is hard to see whether the change of this default setting will affect instances that has been initialized. https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclient.defaultproxy?view=netcore-3.1

As a result, what do you think about the changing of the implementation of Http class to HttpClient @jjw24?

[jjw24]

I have manually checked the error with debugging. The exception is actually "The operation out of time", while when I move the timeout setting up to 3000, sometimes the request can work for me.
However, it is still unstable for me.

Since in our Http class, it utilizes the httpwebrequest to create request, I tried to change it to the new HttpClient, and it works fine. That's seems strange to me, and I have no idea why it can solve the exception.

Besides, at the time I am searching for the utility of HttpClient, there are quite a lot of discussion suggesting that we should not create httpclient instance every time to request. Instead, it should be created for once or get from IHttpClientFactory registered in service. I am not quite sure how that should be done in Flow. That seems may make it harder to change the Proxy setting comparing the implementation now.
It seems that we can just change the global setting of proxy to change proxy setting. However, this require testing since it is hard to see whether the change of this default setting will affect instances that has been initialized. https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclient.defaultproxy?view=netcore-3.1

As a result, what do you think about the changing of the implementation of Http class to HttpClient @jjw24?

Let me investigate as well and get back to you around this. I do agree the use of Factory Method design pattern but I feel it's a stretch for using it here where we have a plugin system.

[jjw24]

Since version 1.2.0 there has been added messages to notify the update progress. Let me know if they are sufficient.

Well, since I haven't successfully updated or checked for update, I haven't seen the message ever. My point is that it would be better to notice users that the update is failing, since I have checked the code, and when catching error, the updater will only provide a log but not giving message to user.

catch (Exception e) when (e is HttpRequestException || e is WebException || e is SocketException) { Log.Exception($"|Updater.UpdateApp|Please check your connection and proxy settings to api.github.com.", e); return; }

Yep thanks for letting me know, makes absolute sense that we should build something in to let users know it has failed. I will put in an issue for this.

[taooceros]

Let me investigate as well and get back to you around this. I do agree the use of Factory Method design pattern but I feel it's a stretch for using it here where we have a plugin system.

I think the plugin system won't be an issue of using the Factory Method design pattern. However, since Flow is not a WebApi program, injection of the Factory will be a little bit complex. However, I have checked the place the Proxy in Flow.Infranstrure.Http change. It seems that we only set the proxy in startup. If that's the case, I think one HttpClient instance will satisfy the need, by passing it with the correct proxy, and we won't need the Factory injection, which is a little messy.

If we want to change the proxy in dynamic way, I have found some solutions solving this by changing the WebProxy instance Address and credential after it has been passed to the HttpClientHandler (or SocketsHttpHandler). That requires more testing.

[jjw24]

Let me investigate as well and get back to you around this. I do agree the use of Factory Method design pattern but I feel it's a stretch for using it here where we have a plugin system.

I think the plugin system won't be an issue of using the Factory Method design pattern. However, since Flow is not a WebApi program, injection of the Factory will be a little bit complex. However, I have checked the place the Proxy in Flow.Infranstrure.Http change. It seems that we only set the proxy in startup. If that's the case, I think one HttpClient instance will satisfy the need, by passing it with the correct proxy, and we won't need the Factory injection, which is a little messy.

If we want to change the proxy in dynamic way, I have found some solutions solving this by changing the WebProxy instance Address and credential after it has been passed to the HttpClientHandler (or SocketsHttpHandler). That requires more testing.

Makes sense. Do you think you can put in a PR so we can have a play with it as well?

[taooceros]

Sure, give me a little bit time to create a new branch since I am not quite familier with Git.

[taooceros]

I have checked squirrel.windows, and it seems that they have natively supported Delta update, which only replace the code that has changed. However, I am not sure why Flow doesn't support that.

[jjw24]

Now that you reminded me, I do remember squirrel has that functionality to only pull diffs, but never had the time to look into it, just wasn't too high on my priority list