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

Don't retry too much in Octokit.fsx #942

Merged
merged 2 commits into from
Sep 11, 2015
Merged

Conversation

vbfox
Copy link
Contributor

@vbfox vbfox commented Sep 8, 2015

This PR changes 2 things in OctoKit.fsx:

  • It avoid retrying hundred of times when chaining operations (4 successive operations like in project scaffold was retrying more than 200 times)
  • It doesn't even retry if the error is an authorization error (Invalid password, login require 2-factor auth, token can't do that, ...) as there is no chance that retrying changes the ouput.

When multiple calls where chained the library did a lot of retries.
For example with the default ProjectScaffold build script it made
more than 200 calls.
If the user isn't authorized to do something or failed authentication
there is no chance that retrying will solve the problem.
forki added a commit that referenced this pull request Sep 11, 2015
Don't retry too much in Octokit.fsx
@forki forki merged commit d5080d6 into fsprojects:master Sep 11, 2015
@forki
Copy link
Member

forki commented Sep 11, 2015

thx a lot

@forki
Copy link
Member

forki commented Sep 11, 2015

I had to revert this for now, since it breaks my release process somehow:

image

anay ideas?

@vbfox
Copy link
Contributor Author

vbfox commented Sep 11, 2015

First impression is that it's a normal Http error and the code failed after 4 retries but I'll test it a little more to see if I can reproduce.

I'll send a PR before that adding logging to retries without any of the things I changed there so we can see how many times a normal request is retried (like NuGet push is doing). Might help with a diagnostic.

@forki
Copy link
Member

forki commented Sep 11, 2015

yes seems even if I revert I get issues. At least your version catches the exception and fails properly.
So I reapply your chanegs and need to investigate

@forki
Copy link
Member

forki commented Sep 11, 2015

/cc @shiftkey @haacked did you change something in Octokit? Always getting

 System.Net.Http.HttpRequestException: Error while copying content to a stream.

@haacked
Copy link

haacked commented Sep 11, 2015

@forki Nothing i can think of that would affect this. Have you tried to nail it down? What are you doing?

@forki
Copy link
Member

forki commented Sep 12, 2015

It seems it only happens on https://github.com/fsprojects/Paket/releases

my other release builds still seem to work. (it's always using the same approach). Maybe I hit some upload limit in Paket (we already have 833 releases)?

@forki
Copy link
Member

forki commented Sep 14, 2015

@haacked this is now happening on more and more projects even when I don't update any Octokit related code. Seems to be an issue with the response from github API. Did it change?

@haacked
Copy link

haacked commented Sep 14, 2015

@forki hmm, might want to contact Support: https://github.com/contact
I personally don't know of any change that would affect this.

@forki
Copy link
Member

forki commented Sep 14, 2015

ok thx. will try to create a plain C# + octokit repro tommorow.

@shiftkey
Copy link
Contributor

shiftkey commented Oct 3, 2015

Did this end up being related to the fix made in octokit/octokit.net#895?

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.

4 participants