Azure / azure-sdk-for-rust

This repository is for active development of the *unofficial* Azure SDK for Rust. This repository is *not* supported by the Azure SDK team.
MIT License
676 stars 231 forks source link

Rename lock_expiry to timeout #1651

Closed Gaelik-git closed 1 month ago

Gaelik-git commented 2 months ago

lock_expiry is not correctly named, and give incorrect indication about the behaviour. I could not found documentation about this on azure but my experiences show that it represents the time after which the http connection send 204 empty response if no message is available

When using a timeout of 100s I get the following debug log from hyper (reqwest)

2024-04-26T15:17:20.027847Z DEBUG app::worker: Fetching message from queue
  at src/worker/mod.rs:46

2024-04-26T15:17:20.028454Z DEBUG hyper::client::connect::dns: resolving host="name.servicebus.windows.net"
  at /Users/baptiste/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-0.14.28/src/client/connect/dns.rs:122

2024-04-26T15:17:20.075441Z DEBUG hyper::client::connect::http: connecting to 23.102.0.186:443
  at /Users/baptiste/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-0.14.28/src/client/connect/http.rs:542

2024-04-26T15:17:20.108438Z DEBUG hyper::client::connect::http: connected to 23.102.0.186:443
  at /Users/baptiste/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-0.14.28/src/client/connect/http.rs:545

2024-04-26T15:17:20.181322Z DEBUG hyper::proto::h1::io: flushed 356 bytes
  at /Users/baptiste/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-0.14.28/src/proto/h1/io.rs:340

// Waiting 100s

2024-04-26T15:19:00.302686Z DEBUG hyper::proto::h1::io: parsed 5 headers
  at /Users/baptiste/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-0.14.28/src/proto/h1/io.rs:208

2024-04-26T15:19:00.302737Z DEBUG hyper::proto::h1::conn: incoming body is empty
  at /Users/baptiste/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-0.14.28/src/proto/h1/conn.rs:224

2024-04-26T15:19:00.302921Z DEBUG hyper::client::pool: pooling idle connection for ("https", name.servicebus.windows.net)
  at /Users/baptiste/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-0.14.28/src/client/pool.rs:380

2024-04-26T15:19:00.303130Z DEBUG app::worker: No Message in Queue
  at src/worker/mod.rs:50

On this api doc 204 response is described as No messages available within the specified timeout period.

Gaelik-git commented 2 months ago

I agree timeout is not a perfect name but lock_expiry is just incorrect. I tried to find the same implementation in other SDK (go, js or java) but they I think they all use the amqp api not the http api so it's difficult to compare

Edit:

In the python SDK we have this description for the timeout

timeout: Optional. Timeout for the http request, in seconds

https://github.com/primecloud-controller-org/azure-sdk-for-python/blob/a6ff7ae21c8a59dd75fbcd67a481ff99e0302e73/azure/servicebus/servicebusservice.py#L102