dasch / avro_turf

A library that makes it easier to use the Avro serialization format from Ruby.
MIT License
167 stars 80 forks source link

Excon's connect_timeout and dns_timeout #200

Closed bruno-b-martins closed 1 year ago

bruno-b-martins commented 1 year ago

Hey there 👋

In a recent PR (#197) we've introduced the connect_timeout option to set a timeout for a connection to the schema registry.

However, that only works for the connection between the application server (where AvroTurf is running) and the destination server (where the schema registry is running). There is another server which is queried -> the domain name server (DNS) that resolves the host into an IP address.

Excon recently (today) released a version that supports setting this timeout towards the dns server (https://github.com/excon/excon/blob/master/README.md?plain=1#L198). I'll open up a PR tomorrow to make use of this option, however, I want to ask you (@dasch) if you'd rather have an additional parameter being passed down from AvroTurf::Messaging to Excon, or use the same connect_timeout we've recently introduced?

Please note that dns_timeouts accepts an integer and an array of integers (to retry the request with different timeouts), while connect_timeout only accepts an integer.

dasch commented 1 year ago

I think it's best to hardcode this until such a time as users need to customize, then we can consider.

bruno-b-martins commented 1 year ago

Ok. Then, I'll simply pass AvroTurf::Messaging's connect_timeout option to Excon's dns_timeouts as well.

Thanks for the feedback.