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

Quickstart test: fail when server responds with error #759

Merged
merged 4 commits into from
Jun 5, 2017

Conversation

gr2m
Copy link
Member

@gr2m gr2m commented Jun 3, 2017

closes #749

@gr2m
Copy link
Member Author

gr2m commented Jun 3, 2017

@janl that worked: https://travis-ci.org/hoodiehq/hoodie/builds/239092845. But it still tries it 60 times instead of stop trying after the first fail.

Maybe there is no workaround for it because for the first few tries the server might not be up yet. I’d say this is good enough now. I’ll put in the workarounds from #758 so that the quickstart test passes again

@gr2m
Copy link
Member Author

gr2m commented Jun 3, 2017

This is ready for review, ping @hoodiehq/maintainers

@janl
Copy link
Member

janl commented Jun 3, 2017

man curl again:

       --retry <num>
              If a transient error is returned when curl tries to perform a transfer, it will
              retry this number of times before giving up. Setting the number to 0 makes curl
              do  no retries (which is the default). Transient error means either: a timeout,
              an FTP 4xx response code or an HTTP 5xx response code.

              When curl is about to retry a transfer, it will first wait one second and  then
              for all forthcoming retries it will double the waiting time until it reaches 10
              minutes which then will be the delay between the rest of the retries.  By using
              --retry-delay you disable this exponential backoff algorithm. See also --retry-
              max-time to limit the total time allowed for retries. (Added in 7.12.3)

              If this option is used several times, the last one will be used.

       --retry-delay <seconds>
              Make curl sleep this amount of time before  each  retry  when  a  transfer  has
              failed  with  a  transient error (it changes the default backoff time algorithm
              between retries). This option is only interesting if --retry is also used. Set-
              ting this delay to zero will make curl use the default backoff time.  (Added in
              7.12.3)

              If this option is used several times, the last one will be used.

       --retry-max-time <seconds>
              The retry timer is reset before the first transfer  attempt.  Retries  will  be
              done  as  usual  (see  --retry)  as long as the timer hasn't reached this given
              limit. Notice that if the timer hasn't reached the limit, the request  will  be
              made  and  while performing, it may take longer than this given time period. To
              limit a single request's maximum time, use -m, --max-time.  Set this option  to
              zero to not timeout retries. (Added in 7.12.3)

              If this option is used several times, the last one will be used.

@gr2m
Copy link
Member Author

gr2m commented Jun 3, 2017

curl --retry 60 http://localhost:8080 fails right away without retrying. Am I doing something wrong?

$ curl --retry 60 http://localhost:8080
curl: (7) Failed to connect to localhost port 8080: Connection refused

@janl
Copy link
Member

janl commented Jun 3, 2017

If a transient error is returned when curl tries to perform a transfer, it will retry this number of times before giving up.

&

Transient error means either: a timeout, an FTP 4xx response code or an HTTP 5xx response code.

Together this means these settings won’t apply when the host is not reachable, but only when you get a timeout, or HTTP 5xx response. But not for a connection refused, or 4xx error.

@gr2m
Copy link
Member Author

gr2m commented Jun 3, 2017

seems like curl can’t handle connection refused errors yet. It was brought up at curl/curl#1036, but was not yet implemented as it seems. Probably that’s why @benwhite-deltas implemented the manual loop :)

@gr2m
Copy link
Member Author

gr2m commented Jun 3, 2017

@janl I added a change: 4ab9daf

That way we have the loop that waits until the server starts. Only then we check if the client bundling works. What do you think?

@gr2m
Copy link
Member Author

gr2m commented Jun 5, 2017

ping @hoodiehq/maintainers this is ready for review

@gr2m gr2m force-pushed the 749/ci-fail-on-quickstart-test-fail branch from 4ab9daf to 524276f Compare June 5, 2017 11:43
Copy link
Member

@janl janl left a comment

Choose a reason for hiding this comment

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

+1

@gr2m gr2m merged commit 64bb2ee into master Jun 5, 2017
@gr2m gr2m deleted the 749/ci-fail-on-quickstart-test-fail branch June 5, 2017 17:03
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.

CI: test pass even though bin/quick-start-test.sh fails in before_script
2 participants