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

More resilient schema lookup #9

Closed
wants to merge 1 commit into from
Closed

Conversation

sparrovv
Copy link
Contributor

It happened to me once or twice that specs were running for a long time,
and then they've failed b/c there was a problem with iglucentral
connection.

I don't have the exception at hand, and couldn't reproduce it now, but I believe it was a flaky network connection or maybe iglucentral wasn't available. I've added read_timeout and simple retry in case it happens again.

Hopefully this will make it more robust.

It happened to me once or twice that specs were running for a long time,
and then it didn't pass b/c there was a problem with iglucentral
connection.

I don't have the exception at hand, and couldn't reproduce it, but I believe it was a flaky network
connection or maybe iglucentral wasn't available. I've added `read_timeout` and simple retry in case it happens again.

Hopefully this will make it more robust.
@chuwy
Copy link
Contributor

chuwy commented Aug 14, 2017

Thanks again, @sparrovv! This also makes it more consistent with canonical Scala Iglu client.

@chuwy chuwy self-requested a review August 14, 2017 12:17
@snowplowcla
Copy link

@sparrovv has signed the Software Grant and Corporate Contributor License Agreement

begin
response = HTTParty.get(schema_uri)
response = HTTParty.get(schema_uri, timeout: 3)
rescue SocketError => _
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm just thinking that we might also want to retry on the SocketError 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a strong opinion on that. But leaning towards not retrying on the SocketError as this is rarely something that can be fixed by bigger delay.

@pvdb
Copy link

pvdb commented Sep 28, 2017

Why only catch and retry on Net::ReadTimeout but not Net::ConnectTimeout?

@alexanderdean alexanderdean added this to the Version 0.2.0 milestone Oct 20, 2017
@chuwy chuwy mentioned this pull request Nov 1, 2017
@chuwy chuwy removed this from the Version 0.2.0 milestone Nov 1, 2017
@oguzhanunlu
Copy link
Member

Cherry-picked to #14 . Closing.

@oguzhanunlu oguzhanunlu closed this Nov 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants