Skip to content

Conversation

@Dzejkop
Copy link
Contributor

@Dzejkop Dzejkop commented Sep 10, 2018

No description provided.

Copy link
Contributor

@genail genail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add an entry in changelog

Copy link
Contributor

@witcher112 witcher112 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you provide some resources/articles, how this will fix the issue?

@Dzejkop
Copy link
Contributor Author

Dzejkop commented Sep 11, 2018

@witcher112 according to MSDN

"Specifically, the ReadWriteTimeout property controls the time-out for the Read method, which is used to read the stream returned by the GetResponseStream method"

@witcher112
Copy link
Contributor

The default value is 300,000 milliseconds (5 minutes).

As I can see, we are setting it for less. Are you sure it is fixing our issue?

@Dzejkop
Copy link
Contributor Author

Dzejkop commented Sep 11, 2018

It's partially fixing the issue. With this fix the Patcher won't freeze for 5 minutes, instead it'll crash the download and (hopefully) retry.

@genail
Copy link
Contributor

genail commented Sep 11, 2018

@Dzejkop did you test it with the script that I've provided?

@witcher112
Copy link
Contributor

Never mind, it was my fault of wrong understanding of this issue. The fix is all right.

@Dzejkop
Copy link
Contributor Author

Dzejkop commented Sep 11, 2018

@genail yes I did to the extent of the download timing out and nothing more

@genail
Copy link
Contributor

genail commented Sep 11, 2018

@Dzejkop just a changelog and we can proceed.

@genail genail merged commit 2e33526 into dev/v3.10.x Sep 11, 2018
@genail genail deleted the bugfix/v3.10.x/1058-added-ReadWriteTimeout-to-patchkit-network branch September 11, 2018 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants