Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Increase the binary fetch request timeout #1695

Merged
merged 1 commit into from
Sep 2, 2016

Conversation

xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Sep 1, 2016

The timeout applies to both the initial connection as well as
individual packets.

I found the 1s timeout to be really fragile when testing locally.
The intent of the timeout is to prevent the install process hanging
indefinitely. It makes sense to be very generous so this increases
the timeout to 60s.

I found the 1s timeout to be really fragile when testing locally.
The intent of the timeout is to prevent the install process hanging
indefinitely. It makes sense to be very generous so this increases
the timeout to 60s.

The timeout applies to both the initial connection as well as
individual packets.
@nschonni
Copy link
Contributor

nschonni commented Sep 1, 2016

The test failures from Travis are odd, but don't seem related

@xzyfer
Copy link
Contributor Author

xzyfer commented Sep 2, 2016

Yeah looks to be related to tests not cleaning after themselves + retries. I mentioned this could happen in #1692. I'll look at making the tests that touch the FS reentrant. IIRC @saper had some thoughts about this.

@xzyfer xzyfer merged commit 7bf34be into sass:master Sep 2, 2016
@xzyfer xzyfer deleted the feat/increase-timeout branch September 2, 2016 04:38
@nschonni
Copy link
Contributor

nschonni commented Sep 2, 2016

Maybe just move them to the allowed failures section? https://docs.travis-ci.com/user/customizing-the-build/#Rows-that-are-Allowed-to-Fail

@xzyfer
Copy link
Contributor Author

xzyfer commented Sep 2, 2016

They shouldn't fail though. Specifically it happens because the test creates a file in a temp dir, then spawn errors for an unrelated reason which causes a retry. On the second attempt the file is still there cause the create file step to error. Now that we know we can catch the spawn failures we can make sure to clean up after our selves before retrying.

@xzyfer xzyfer modified the milestone: next.patch Sep 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants